You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "cashmand (via GitHub)" <gi...@apache.org> on 2025/02/12 18:17:43 UTC
[PR] Link to Parquet spec. [spark]
cashmand opened a new pull request, #49910:
URL: https://github.com/apache/spark/pull/49910
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error type or message, please read the guideline first in
'common/utils/src/main/resources/error/README.md'.
-->
### What changes were proposed in this pull request?
The Parquet spec in https://github.com/apache/parquet-format/blob/master/VariantEncoding.md is based on the one in Spark, but has received a number of updates (especially related to Shredding).
At this point, the code in Spark more closely matches the latest version of the Parquet spec (the main gap being the lack of a few new scalar types that were recently added, and which we will try to add to Spark soon).
This PR updates the README.md and shredding.md files to just point to the Parquet spec, which we plan to have the Spark code follow.
### Why are the changes needed?
Improve internal documentation and avoid maintaining two copies of the spec.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
None.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51183][SQL] Link to Parquet spec in Variant docs [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #49910:
URL: https://github.com/apache/spark/pull/49910#issuecomment-2655015074
Could you send an email to the Apache Spark dev mailing list because this is the status of our previous decision, @cashmand and @gene-db ?
Although we are in the out-of-sync status, it's worthy to share the AS-IS status in order to avoid any surprise after Apache Spark 4.0.0 release. It would be great to be as transparent as possible for not only this PR, but also for
- https://github.com/apache/spark/pull/49874 .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51183][SQL] Link to Parquet spec in Variant docs [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #49910:
URL: https://github.com/apache/spark/pull/49910#discussion_r1953217184
##########
common/variant/README.md:
##########
@@ -1,379 +1 @@
-# Overview
-
-A Variant represents a type that contain one of:
-- Primitive: A type and corresponding value (e.g. INT, STRING)
-- Array: An ordered list of Variant values
-- Object: An unordered collection of string/Variant pairs (i.e. key/value pairs). An object may not contain duplicate keys.
-
-A variant is encoded with 2 binary values, the [value](#value-encoding) and the [metadata](#metadata-encoding).
-
-There are a fixed number of allowed primitive types, provided in the table below. These represent a commonly supported subset of the [logical types](https://github.com/apache/parquet-format/blob/master/LogicalTypes.md) allowed by the Parquet.
-
-The Variant spec allows representation of semi-structured data (e.g. JSON) in a form that can be efficiently queried by path. The design is intended to allow efficient access to nested data even in the presence of very wide or deep structures.
-
-Another motivation for the representation is that (aside from metadata) each inner Variant value is contiguous and self-contained. For example, in a Variant containing an Array of Variant values, the representation of an inner Variant value, when paired with the metadata of the full variant, is itself a valid Variant.
-
-# Metadata encoding
-
-The encoded metadata always starts with a header byte.
-```
- 7 6 5 4 3 0
- +-------+---+---+---------------+
-header | | | | version |
- +-------+---+---+---------------+
- ^ ^
- | +-- sorted_strings
- +-- offset_size_minus_one
-```
-The `version` is a 4-bit value that must always contain the value `1`.
-`sorted_strings` is a 1-bit value indicating whether dictionary strings are sorted and unique.
-`offset_size_minus_one` is a 2-bit value providing the number of bytes per dictionary size and offset field.
-The actual number of bytes, `offset_size`, is `offset_size_minus_one + 1`.
-
-The entire metadata is encoded as the following diagram shows:
-```
- 7 0
- +-----------------------+
-metadata | header |
- +-----------------------+
- | |
- : dictionary_size : <-- little-endian, `offset_size` bytes
- | |
- +-----------------------+
- | |
- : offset : <-- little-endian, `offset_size` bytes
- | |
- +-----------------------+
- :
- +-----------------------+
- | |
- : offset : <-- little-endian, `offset_size` bytes
- | | (`dictionary_size + 1` offsets)
- +-----------------------+
- | |
- : bytes :
- | |
- +-----------------------+
-```
-
-The metadata is encoded first with the `header` byte, then `dictionary_size` which is a little-endian value of `offset_size` bytes, and represents the number of string values in the dictionary.
-Next, is an `offset` list, which contains `dictionary_size + 1` values.
-Each `offset` is a little-endian value of `offset_size` bytes, and represents the starting byte offset of the i-th string in `bytes`.
-The first `offset` value will always be `0`, and the last `offset` value will always be the total length of `bytes`.
-The last part of the metadata is `bytes`, which stores all the string values in the dictionary.
-
-## Metadata encoding grammar
-
-The grammar for encoded metadata is as follows
-
-```
-metadata: <header> <dictionary_size> <dictionary>
-header: 1 byte (<version> | <sorted_strings> << 4 | (<offset_size_minus_one> << 6))
-version: a 4-bit version ID. Currently, must always contain the value 1
-sorted_strings: a 1-bit value indicating whether metadata strings are sorted
-offset_size_minus_one: 2-bit value providing the number of bytes per dictionary size and offset field.
-dictionary_size: `offset_size` bytes. little-endian value indicating the number of strings in the dictionary
-dictionary: <offset>* <bytes>
-offset: `offset_size` bytes. little-endian value indicating the starting position of the ith string in `bytes`. The list should contain `dictionary_size + 1` values, where the last value is the total length of `bytes`.
-bytes: dictionary string values
-```
-
-Notes:
-- Offsets are relative to the start of the `bytes` array.
-- The length of the ith string can be computed as `offset[i+1] - offset[i]`.
-- The offset of the first string is always equal to 0 and is therefore redundant. It is included in the spec to simplify in-memory-processing.
-- `offset_size_minus_one` indicates the number of bytes per `dictionary_size` and `offset` entry. I.e. a value of 0 indicates 1-byte offsets, 1 indicates 2-byte offsets, 2 indicates 3 byte offsets and 3 indicates 4-byte offsets.
-- If `sorted_strings` is set to 1, strings in the dictionary must be unique and sorted in lexicographic order. If the value is set to 0, readers may not make any assumptions about string order or uniqueness.
-
-
-# Value encoding
-
-The entire encoded Variant value includes the `value_metadata` byte, and then 0 or more bytes for the `val`.
-```
- 7 2 1 0
- +------------------------------------+------------+
-value | value_header | basic_type |
- +------------------------------------+------------+
- | |
- : value_data : <-- 0 or more bytes
- | |
- +-------------------------------------------------+
-```
-## Basic Type
-
-The `basic_type` is 2-bit value that represents which basic type the Variant value is.
-The [basic types table](#encoding-types) shows what each value represents.
-
-## Value Header
-
-The `value_header` is a 6-bit value that contains more information about the type, and the format depends on the `basic_type`.
-
-### Value Header for Primitive type (`basic_type`=0)
-
-When `basic_type` is `0`, `value_header` is a 6-bit `primitive_header`.
-The [primitive types table](#encoding-types) shows what each value represents.
-```
- 5 0
- +-----------------------+
-value_header | primitive_header |
- +-----------------------+
-```
-
-### Value Header for Short string (`basic_type`=1)
-
-When `basic_type` is `1`, `value_header` is a 6-bit `short_string_header`.
-```
- 5 0
- +-----------------------+
-value_header | short_string_header |
- +-----------------------+
-```
-The `short_string_header` value is the length of the string.
-
-### Value Header for Object (`basic_type`=2)
-
-When `basic_type` is `2`, `value_header` is made up of `field_offset_size_minus_one`, `field_id_size_minus_one`, and `is_large`.
-```
- 5 4 3 2 1 0
- +---+---+-------+-------+
-value_header | | | | |
- +---+---+-------+-------+
- ^ ^ ^
- | | +-- field_offset_size_minus_one
- | +-- field_id_size_minus_one
- +-- is_large
-```
-`field_offset_size_minus_one` and `field_id_size_minus_one` are 2-bit values that represent the number of bytes used to encode the field offsets and field ids.
-The actual number of bytes is computed as `field_offset_size_minus_one + 1` and `field_id_size_minus_one + 1`.
-`is_large` is a 1-bit value that indicates how many bytes are used to encode the number of elements.
-If `is_large` is `0`, 1 byte is used, and if `is_large` is `1`, 4 bytes are used.
-
-### Value Header for Array (`basic_type`=3)
-
-When `basic_type` is `3`, `value_header` is made up of `field_offset_size_minus_one`, and `is_large`.
-```
- 5 3 2 1 0
- +-----------+---+-------+
-value_header | | | |
- +-----------+---+-------+
- ^ ^
- | +-- field_offset_size_minus_one
- +-- is_large
-```
-`field_offset_size_minus_one` is a 2-bit value that represents the number of bytes used to encode the field offset.
-The actual number of bytes is computed as `field_offset_size_minus_one + 1`.
-`is_large` is a 1-bit value that indicates how many bytes are used to encode the number of elements.
-If `is_large` is `0`, 1 byte is used, and if `is_large` is `1`, 4 bytes are used.
-
-## Value Data
-
-The `value_data` encoding format depends on the type specified by `value_metadata`.
-For some types, the `value_data` will be 0-bytes.
-
-### Value Data for Primitive type (`basic_type`=0)
-
-When `basic_type` is `0`, `value_data` depends on the `primitive_header` value.
-The [primitive types table](#encoding-types) shows the encoding format for each primitive type.
-
-### Value Data for Short string (`basic_type`=1)
-
-When `basic_type` is `1`, `value_data` is the sequence of bytes that represents the string.
-
-### Value Data for Object (`basic_type`=2)
-
-When `basic_type` is `2`, `value_data` encodes an object.
-The encoding format is shown in the following diagram:
-```
- 7 0
- +-----------------------+
-object value_data | |
- : num_elements : <-- little-endian, 1 or 4 bytes
- | |
- +-----------------------+
- | |
- : field_id : <-- little-endian, `field_id_size` bytes
- | |
- +-----------------------+
- :
- +-----------------------+
- | |
- : field_id : <-- little-endian, `field_id_size` bytes
- | | (`num_elements` field_ids)
- +-----------------------+
- | |
- : field_offset : <-- little-endian, `field_offset_size` bytes
- | |
- +-----------------------+
- :
- +-----------------------+
- | |
- : field_offset : <-- little-endian, `field_offset_size` bytes
- | | (`num_elements + 1` field_offsets)
- +-----------------------+
- | |
- : value :
- | |
- +-----------------------+
- :
- +-----------------------+
- | |
- : value : <-- (`num_elements` values)
- | |
- +-----------------------+
-```
-An object `value_data` begins with `num_elements`, a 1-byte or 4-byte little-endian value, representing the number of elements in the object.
-The size in bytes of `num_elements` is indicated by `is_large` in the `value_header`.
-Next, is a list of `field_id` values.
-There are `num_elements` number of entries and each `field_id` is a little-endian value of `field_id_size` bytes.
-A `field_id` is an index into the dictionary in the metadata.
-The `field_id` list is followed by a `field_offset` list.
-There are `num_elements + 1` number of entries and each `field_offset` is a little-endian value of `field_offset_size` bytes.
-A `field_offset` represents the byte offset (relative to the first byte of the first `value`) where the i-th `value` starts.
-The last `field_offset` points to the byte after the end of the last `value`.
-The `field_offset` list is followed by the `value` list.
-There are `num_elements` number of `value` entries and each `value` is an encoded Variant value.
-For the i-th key-value pair of the object, the key is the metadata dictionary entry indexed by the i-th `field_id`, and the value is the Variant `value` starting from the i-th `field_offset` byte offset.
-
-The field ids and field offsets must be in lexicographical order of the corresponding field names in the metadata dictionary.
-However, the actual `value` entries do not need to be in any particular order.
-This implies that the `field_offset` values may not be monotonically increasing.
-For example, for the following object:
-```
-{
- "c": 3,
- "b": 2,
- "a": 1
-}
-```
-The `field_id` list must be `[<id for key "a">, <id for key "b">, <id for key "c">]`, in lexicographical order.
-The `field_offset` list must be `[<offset for value 1>, <offset for value 2>, <offset for value 3>, <last offset>]`.
-The `value` list can be in any order.
-
-### Value Data for Array (`basic_type`=3)
-
-When `basic_type` is `3`, `value_data` encodes an array. The encoding format is shown in the following diagram:
-```
- 7 0
- +-----------------------+
-array value_data | |
- : num_elements : <-- little-endian, 1 or 4 bytes
- | |
- +-----------------------+
- | |
- : field_offset : <-- little-endian, `field_offset_size` bytes
- | |
- +-----------------------+
- :
- +-----------------------+
- | |
- : field_offset : <-- little-endian, `field_offset_size` bytes
- | | (`num_elements + 1` field_offsets)
- +-----------------------+
- | |
- : value :
- | |
- +-----------------------+
- :
- +-----------------------+
- | |
- : value : <-- (`num_elements` values)
- | |
- +-----------------------+
-```
-An array `value_data` begins with `num_elements`, a 1-byte or 4-byte little-endian value, representing the number of elements in the array.
-The size in bytes of `num_elements` is indicated by `is_large` in the `value_header`.
-Next, is a `field_offset` list.
-There are `num_elements + 1` number of entries and each `field_offset` is a little-endian value of `field_offset_size` bytes.
-A `field_offset` represents the byte offset (relative to the first byte of the first `value`) where the i-th `value` starts.
-The last `field_offset` points to the byte after the last byte of the last `value`.
-The `field_offset` list is followed by the `value` list.
-There are `num_elements` number of `value` entries and each `value` is an encoded Variant value.
-For the i-th array entry, the value is the Variant `value` starting from the i-th `field_offset` byte offset.
-
-## Value encoding grammar
-
-The grammar for an encoded value is:
-
-```
-value: <value_metadata> <value_data>?
-value_metadata: 1 byte (<basic_type> | (<value_header> << 2))
-basic_type: ID from Basic Type table. <value_header> must be a corresponding variation
-value_header: <primitive_header> | <short_string_header> | <object_header> | <array_header>
-primitive_header: ID from Primitive Type table. <val> must be a corresponding variation of <primitive_val>
-short_string_header: unsigned string length in bytes from 0 to 63
-object_header: (is_large << 4 | field_id_size_minus_one << 2 | field_offset_size_minus_one)
-array_header: (is_large << 2 | field_offset_size_minus_one)
-value_data: <primitive_val> | <short_string_val> | <object_val> | <array_val>
-primitive_val: see table for binary representation
-short_string_val: bytes
-object_val: <num_elements> <field_id>* <field_offset>* <fields>
-array_val: <num_elements> <field_offset>* <fields>
-num_elements: a 1 or 4 byte little-endian value (depending on is_large in <object_header>/<array_header>)
-field_id: a 1, 2, 3 or 4 byte little-endian value (depending on field_id_size_minus_one in <object_header>), indexing into the dictionary
-field_offset: a 1, 2, 3 or 4 byte little-endian value (depending on field_offset_size_minus_one in <object_header>/<array_header>), providing the offset in bytes within fields
-fields: <value>*
-```
-
-Each `value_data` must correspond to the type defined by `value_metadata`. Boolean and null types do not have a corresponding `value_data`, since their type defines their value.
-
-Each `array_val` and `object_val` must contain exactly `num_elements + 1` values for `field_offset`. The last entry is the offset that is one byte past the last field (i.e. the total size of all fields in bytes). All offsets are relative to the first byte of the first field in the object/array.
-
-`field_id_size_minus_one` and `field_offset_size_minus_one` indicate the number of bytes per field ID/offset. I.e. a value of 0 indicates 1-byte IDs, 1 indicates 2-byte IDs, 2 indicates 3 byte IDs and 3 indicates 4-byte IDs. The `is_large` flag for arrays and objects is used to indicate whether the number of elements is indicated using a one or four byte value. When more than 255 elements are present, `is_large` must be set to true. It is valid for an implementation to use a larger value than necessary for any of these fields (e.g. `is_large` may be true for an object with less than 256 elements).
-
-The "short string" basic type may be used as an optimization to fold string length into the type byte for strings less than 64 bytes. It is semantically identical to the "string" primitive type.
-
-The Decimal type contains a scale, but no precision. The implied precision of a decimal value is `floor(log_10(val)) + 1`.
-
-# Encoding types
-
-| Basic Type | ID | Description |
-|--------------|-----|---------------------------------------------------|
-| Primitive | `0` | One of the primitive types |
-| Short string | `1` | A string with a length less than 64 bytes |
-| Object | `2` | A collection of (string-key, variant-value) pairs |
-| Array | `3` | An ordered sequence of variant values |
-
-| Logical Type | Physical Type | Type ID | Equivalent Parquet Type | Binary format |
-|----------------------|-----------------------------|---------|-----------------------------|---------------------------------------------------------------------------------------------------------------------|
-| NullType | null | `0` | any | none |
-| Boolean | boolean (True) | `1` | BOOLEAN | none |
-| Boolean | boolean (False) | `2` | BOOLEAN | none |
-| Exact Numeric | int8 | `3` | INT(8, signed) | 1 byte |
-| Exact Numeric | int16 | `4` | INT(16, signed) | 2 byte little-endian |
-| Exact Numeric | int32 | `5` | INT(32, signed) | 4 byte little-endian |
-| Exact Numeric | int64 | `6` | INT(64, signed) | 8 byte little-endian |
-| Double | double | `7` | DOUBLE | IEEE little-endian |
-| Exact Numeric | decimal4 | `8` | DECIMAL(precision, scale) | 1 byte scale in range [0, 38], followed by little-endian unscaled value (see decimal table) |
-| Exact Numeric | decimal8 | `9` | DECIMAL(precision, scale) | 1 byte scale in range [0, 38], followed by little-endian unscaled value (see decimal table) |
-| Exact Numeric | decimal16 | `10` | DECIMAL(precision, scale) | 1 byte scale in range [0, 38], followed by little-endian unscaled value (see decimal table) |
-| Date | date | `11` | DATE | 4 byte little-endian |
-| Timestamp | timestamp | `12` | TIMESTAMP(true, MICROS) | 8-byte little-endian |
-| TimestampNTZ | timestamp without time zone | `13` | TIMESTAMP(false, MICROS) | 8-byte little-endian |
-| Float | float | `14` | FLOAT | IEEE little-endian |
-| Binary | binary | `15` | BINARY | 4 byte little-endian size, followed by bytes |
-| String | string | `16` | STRING | 4 byte little-endian size, followed by UTF-8 encoded bytes |
-
-| Decimal Precision | Decimal value type |
-|-----------------------|--------------------|
-| 1 <= precision <= 9 | int32 |
-| 10 <= precision <= 18 | int64 |
-| 18 <= precision <= 38 | int128 |
-| > 38 | Not supported |
-
-The *Logical Type* column indicates logical equivalence of physically encoded types. For example, a user expression operating on a string value containing "hello" should behave the same, whether it is encoded with the short string optimization, or long string encoding. Similarly, user expressions operating on an *int8* value of 1 should behave the same as a decimal16 with scale 2 and unscaled value 100.
-
-# Field ID order and uniqueness
-
-For objects, field IDs and offsets must be listed in the order of the corresponding field names, sorted lexicographically. Note that the fields themselves are not required to follow this order. As a result, offsets will not necessarily be listed in ascending order.
-
-An implementation may rely on this field ID order in searching for field names. E.g. a binary search on field IDs (combined with metadata lookups) may be used to find a field with a given field.
-
-Field names are case-sensitive. Field names are required to be unique for each object. It is an error for an object to contain two fields with the same name, whether or not they have distinct dictionary IDs.
-
-# Versions and extensions
-
-An implementation is not expected to parse a Variant value whose metadata version is higher than the version supported by the implementation. However, new types may be added to the specification without incrementing the version ID. In such a situation, an implementation should be able to read the rest of the Variant value if desired.
-
-# Shredding
-
-For columnar storage formats, a single Variant object may have poor read performance when only a small number of fields are needed. A better approach is to create separate columns for individual fields, referred to as shredding or subcolumnarization. [shredding.md](shredding.md) describes an approach to shredding Variant columns in Parquet and similar columnar formats.
+The Variant type specification has been moved to https://github.com/apache/parquet-format/blob/master/VariantEncoding.md and https://github.com/apache/parquet-format/blob/master/VariantShredding.md.
Review Comment:
Is this a link to `master`? I expected a tag-based link. Is there no release Apache Parquet with
`VariantEncoding.md`?
- https://github.com/apache/parquet-format/tree/apache-parquet-format-2.10.0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51183][SQL] Link to Parquet spec in Variant docs [spark]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #49910:
URL: https://github.com/apache/spark/pull/49910#issuecomment-2655346077
> point to the (not finalized) Variant spec in Parquet (what this PR is doing)
@gene-db I agree with this option. To avoid confusion, the link should point to a git-hash instead of the master branch. It would also be good to leave some brief messages to reflect the current implementation status in Spark's Variant README.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51183][SQL] Link to Parquet spec in Variant docs [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #49910:
URL: https://github.com/apache/spark/pull/49910#issuecomment-2654561078
I'm not sure this is a better idea because we have no tag-based link. In addition, the pointed document shows warnings.
<img width="742" alt="Screenshot 2025-02-12 at 10 42 21" src="https://github.com/user-attachments/assets/31927d48-0c02-49e1-8ce1-f1a4bc9f5783" />
IMO, this doesn't look like a production-ready.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51183][SQL] Link to Parquet spec in Variant docs [spark]
Posted by "cashmand (via GitHub)" <gi...@apache.org>.
cashmand commented on code in PR #49910:
URL: https://github.com/apache/spark/pull/49910#discussion_r1953232672
##########
common/variant/README.md:
##########
@@ -1,379 +1 @@
-# Overview
-
-A Variant represents a type that contain one of:
-- Primitive: A type and corresponding value (e.g. INT, STRING)
-- Array: An ordered list of Variant values
-- Object: An unordered collection of string/Variant pairs (i.e. key/value pairs). An object may not contain duplicate keys.
-
-A variant is encoded with 2 binary values, the [value](#value-encoding) and the [metadata](#metadata-encoding).
-
-There are a fixed number of allowed primitive types, provided in the table below. These represent a commonly supported subset of the [logical types](https://github.com/apache/parquet-format/blob/master/LogicalTypes.md) allowed by the Parquet.
-
-The Variant spec allows representation of semi-structured data (e.g. JSON) in a form that can be efficiently queried by path. The design is intended to allow efficient access to nested data even in the presence of very wide or deep structures.
-
-Another motivation for the representation is that (aside from metadata) each inner Variant value is contiguous and self-contained. For example, in a Variant containing an Array of Variant values, the representation of an inner Variant value, when paired with the metadata of the full variant, is itself a valid Variant.
-
-# Metadata encoding
-
-The encoded metadata always starts with a header byte.
-```
- 7 6 5 4 3 0
- +-------+---+---+---------------+
-header | | | | version |
- +-------+---+---+---------------+
- ^ ^
- | +-- sorted_strings
- +-- offset_size_minus_one
-```
-The `version` is a 4-bit value that must always contain the value `1`.
-`sorted_strings` is a 1-bit value indicating whether dictionary strings are sorted and unique.
-`offset_size_minus_one` is a 2-bit value providing the number of bytes per dictionary size and offset field.
-The actual number of bytes, `offset_size`, is `offset_size_minus_one + 1`.
-
-The entire metadata is encoded as the following diagram shows:
-```
- 7 0
- +-----------------------+
-metadata | header |
- +-----------------------+
- | |
- : dictionary_size : <-- little-endian, `offset_size` bytes
- | |
- +-----------------------+
- | |
- : offset : <-- little-endian, `offset_size` bytes
- | |
- +-----------------------+
- :
- +-----------------------+
- | |
- : offset : <-- little-endian, `offset_size` bytes
- | | (`dictionary_size + 1` offsets)
- +-----------------------+
- | |
- : bytes :
- | |
- +-----------------------+
-```
-
-The metadata is encoded first with the `header` byte, then `dictionary_size` which is a little-endian value of `offset_size` bytes, and represents the number of string values in the dictionary.
-Next, is an `offset` list, which contains `dictionary_size + 1` values.
-Each `offset` is a little-endian value of `offset_size` bytes, and represents the starting byte offset of the i-th string in `bytes`.
-The first `offset` value will always be `0`, and the last `offset` value will always be the total length of `bytes`.
-The last part of the metadata is `bytes`, which stores all the string values in the dictionary.
-
-## Metadata encoding grammar
-
-The grammar for encoded metadata is as follows
-
-```
-metadata: <header> <dictionary_size> <dictionary>
-header: 1 byte (<version> | <sorted_strings> << 4 | (<offset_size_minus_one> << 6))
-version: a 4-bit version ID. Currently, must always contain the value 1
-sorted_strings: a 1-bit value indicating whether metadata strings are sorted
-offset_size_minus_one: 2-bit value providing the number of bytes per dictionary size and offset field.
-dictionary_size: `offset_size` bytes. little-endian value indicating the number of strings in the dictionary
-dictionary: <offset>* <bytes>
-offset: `offset_size` bytes. little-endian value indicating the starting position of the ith string in `bytes`. The list should contain `dictionary_size + 1` values, where the last value is the total length of `bytes`.
-bytes: dictionary string values
-```
-
-Notes:
-- Offsets are relative to the start of the `bytes` array.
-- The length of the ith string can be computed as `offset[i+1] - offset[i]`.
-- The offset of the first string is always equal to 0 and is therefore redundant. It is included in the spec to simplify in-memory-processing.
-- `offset_size_minus_one` indicates the number of bytes per `dictionary_size` and `offset` entry. I.e. a value of 0 indicates 1-byte offsets, 1 indicates 2-byte offsets, 2 indicates 3 byte offsets and 3 indicates 4-byte offsets.
-- If `sorted_strings` is set to 1, strings in the dictionary must be unique and sorted in lexicographic order. If the value is set to 0, readers may not make any assumptions about string order or uniqueness.
-
-
-# Value encoding
-
-The entire encoded Variant value includes the `value_metadata` byte, and then 0 or more bytes for the `val`.
-```
- 7 2 1 0
- +------------------------------------+------------+
-value | value_header | basic_type |
- +------------------------------------+------------+
- | |
- : value_data : <-- 0 or more bytes
- | |
- +-------------------------------------------------+
-```
-## Basic Type
-
-The `basic_type` is 2-bit value that represents which basic type the Variant value is.
-The [basic types table](#encoding-types) shows what each value represents.
-
-## Value Header
-
-The `value_header` is a 6-bit value that contains more information about the type, and the format depends on the `basic_type`.
-
-### Value Header for Primitive type (`basic_type`=0)
-
-When `basic_type` is `0`, `value_header` is a 6-bit `primitive_header`.
-The [primitive types table](#encoding-types) shows what each value represents.
-```
- 5 0
- +-----------------------+
-value_header | primitive_header |
- +-----------------------+
-```
-
-### Value Header for Short string (`basic_type`=1)
-
-When `basic_type` is `1`, `value_header` is a 6-bit `short_string_header`.
-```
- 5 0
- +-----------------------+
-value_header | short_string_header |
- +-----------------------+
-```
-The `short_string_header` value is the length of the string.
-
-### Value Header for Object (`basic_type`=2)
-
-When `basic_type` is `2`, `value_header` is made up of `field_offset_size_minus_one`, `field_id_size_minus_one`, and `is_large`.
-```
- 5 4 3 2 1 0
- +---+---+-------+-------+
-value_header | | | | |
- +---+---+-------+-------+
- ^ ^ ^
- | | +-- field_offset_size_minus_one
- | +-- field_id_size_minus_one
- +-- is_large
-```
-`field_offset_size_minus_one` and `field_id_size_minus_one` are 2-bit values that represent the number of bytes used to encode the field offsets and field ids.
-The actual number of bytes is computed as `field_offset_size_minus_one + 1` and `field_id_size_minus_one + 1`.
-`is_large` is a 1-bit value that indicates how many bytes are used to encode the number of elements.
-If `is_large` is `0`, 1 byte is used, and if `is_large` is `1`, 4 bytes are used.
-
-### Value Header for Array (`basic_type`=3)
-
-When `basic_type` is `3`, `value_header` is made up of `field_offset_size_minus_one`, and `is_large`.
-```
- 5 3 2 1 0
- +-----------+---+-------+
-value_header | | | |
- +-----------+---+-------+
- ^ ^
- | +-- field_offset_size_minus_one
- +-- is_large
-```
-`field_offset_size_minus_one` is a 2-bit value that represents the number of bytes used to encode the field offset.
-The actual number of bytes is computed as `field_offset_size_minus_one + 1`.
-`is_large` is a 1-bit value that indicates how many bytes are used to encode the number of elements.
-If `is_large` is `0`, 1 byte is used, and if `is_large` is `1`, 4 bytes are used.
-
-## Value Data
-
-The `value_data` encoding format depends on the type specified by `value_metadata`.
-For some types, the `value_data` will be 0-bytes.
-
-### Value Data for Primitive type (`basic_type`=0)
-
-When `basic_type` is `0`, `value_data` depends on the `primitive_header` value.
-The [primitive types table](#encoding-types) shows the encoding format for each primitive type.
-
-### Value Data for Short string (`basic_type`=1)
-
-When `basic_type` is `1`, `value_data` is the sequence of bytes that represents the string.
-
-### Value Data for Object (`basic_type`=2)
-
-When `basic_type` is `2`, `value_data` encodes an object.
-The encoding format is shown in the following diagram:
-```
- 7 0
- +-----------------------+
-object value_data | |
- : num_elements : <-- little-endian, 1 or 4 bytes
- | |
- +-----------------------+
- | |
- : field_id : <-- little-endian, `field_id_size` bytes
- | |
- +-----------------------+
- :
- +-----------------------+
- | |
- : field_id : <-- little-endian, `field_id_size` bytes
- | | (`num_elements` field_ids)
- +-----------------------+
- | |
- : field_offset : <-- little-endian, `field_offset_size` bytes
- | |
- +-----------------------+
- :
- +-----------------------+
- | |
- : field_offset : <-- little-endian, `field_offset_size` bytes
- | | (`num_elements + 1` field_offsets)
- +-----------------------+
- | |
- : value :
- | |
- +-----------------------+
- :
- +-----------------------+
- | |
- : value : <-- (`num_elements` values)
- | |
- +-----------------------+
-```
-An object `value_data` begins with `num_elements`, a 1-byte or 4-byte little-endian value, representing the number of elements in the object.
-The size in bytes of `num_elements` is indicated by `is_large` in the `value_header`.
-Next, is a list of `field_id` values.
-There are `num_elements` number of entries and each `field_id` is a little-endian value of `field_id_size` bytes.
-A `field_id` is an index into the dictionary in the metadata.
-The `field_id` list is followed by a `field_offset` list.
-There are `num_elements + 1` number of entries and each `field_offset` is a little-endian value of `field_offset_size` bytes.
-A `field_offset` represents the byte offset (relative to the first byte of the first `value`) where the i-th `value` starts.
-The last `field_offset` points to the byte after the end of the last `value`.
-The `field_offset` list is followed by the `value` list.
-There are `num_elements` number of `value` entries and each `value` is an encoded Variant value.
-For the i-th key-value pair of the object, the key is the metadata dictionary entry indexed by the i-th `field_id`, and the value is the Variant `value` starting from the i-th `field_offset` byte offset.
-
-The field ids and field offsets must be in lexicographical order of the corresponding field names in the metadata dictionary.
-However, the actual `value` entries do not need to be in any particular order.
-This implies that the `field_offset` values may not be monotonically increasing.
-For example, for the following object:
-```
-{
- "c": 3,
- "b": 2,
- "a": 1
-}
-```
-The `field_id` list must be `[<id for key "a">, <id for key "b">, <id for key "c">]`, in lexicographical order.
-The `field_offset` list must be `[<offset for value 1>, <offset for value 2>, <offset for value 3>, <last offset>]`.
-The `value` list can be in any order.
-
-### Value Data for Array (`basic_type`=3)
-
-When `basic_type` is `3`, `value_data` encodes an array. The encoding format is shown in the following diagram:
-```
- 7 0
- +-----------------------+
-array value_data | |
- : num_elements : <-- little-endian, 1 or 4 bytes
- | |
- +-----------------------+
- | |
- : field_offset : <-- little-endian, `field_offset_size` bytes
- | |
- +-----------------------+
- :
- +-----------------------+
- | |
- : field_offset : <-- little-endian, `field_offset_size` bytes
- | | (`num_elements + 1` field_offsets)
- +-----------------------+
- | |
- : value :
- | |
- +-----------------------+
- :
- +-----------------------+
- | |
- : value : <-- (`num_elements` values)
- | |
- +-----------------------+
-```
-An array `value_data` begins with `num_elements`, a 1-byte or 4-byte little-endian value, representing the number of elements in the array.
-The size in bytes of `num_elements` is indicated by `is_large` in the `value_header`.
-Next, is a `field_offset` list.
-There are `num_elements + 1` number of entries and each `field_offset` is a little-endian value of `field_offset_size` bytes.
-A `field_offset` represents the byte offset (relative to the first byte of the first `value`) where the i-th `value` starts.
-The last `field_offset` points to the byte after the last byte of the last `value`.
-The `field_offset` list is followed by the `value` list.
-There are `num_elements` number of `value` entries and each `value` is an encoded Variant value.
-For the i-th array entry, the value is the Variant `value` starting from the i-th `field_offset` byte offset.
-
-## Value encoding grammar
-
-The grammar for an encoded value is:
-
-```
-value: <value_metadata> <value_data>?
-value_metadata: 1 byte (<basic_type> | (<value_header> << 2))
-basic_type: ID from Basic Type table. <value_header> must be a corresponding variation
-value_header: <primitive_header> | <short_string_header> | <object_header> | <array_header>
-primitive_header: ID from Primitive Type table. <val> must be a corresponding variation of <primitive_val>
-short_string_header: unsigned string length in bytes from 0 to 63
-object_header: (is_large << 4 | field_id_size_minus_one << 2 | field_offset_size_minus_one)
-array_header: (is_large << 2 | field_offset_size_minus_one)
-value_data: <primitive_val> | <short_string_val> | <object_val> | <array_val>
-primitive_val: see table for binary representation
-short_string_val: bytes
-object_val: <num_elements> <field_id>* <field_offset>* <fields>
-array_val: <num_elements> <field_offset>* <fields>
-num_elements: a 1 or 4 byte little-endian value (depending on is_large in <object_header>/<array_header>)
-field_id: a 1, 2, 3 or 4 byte little-endian value (depending on field_id_size_minus_one in <object_header>), indexing into the dictionary
-field_offset: a 1, 2, 3 or 4 byte little-endian value (depending on field_offset_size_minus_one in <object_header>/<array_header>), providing the offset in bytes within fields
-fields: <value>*
-```
-
-Each `value_data` must correspond to the type defined by `value_metadata`. Boolean and null types do not have a corresponding `value_data`, since their type defines their value.
-
-Each `array_val` and `object_val` must contain exactly `num_elements + 1` values for `field_offset`. The last entry is the offset that is one byte past the last field (i.e. the total size of all fields in bytes). All offsets are relative to the first byte of the first field in the object/array.
-
-`field_id_size_minus_one` and `field_offset_size_minus_one` indicate the number of bytes per field ID/offset. I.e. a value of 0 indicates 1-byte IDs, 1 indicates 2-byte IDs, 2 indicates 3 byte IDs and 3 indicates 4-byte IDs. The `is_large` flag for arrays and objects is used to indicate whether the number of elements is indicated using a one or four byte value. When more than 255 elements are present, `is_large` must be set to true. It is valid for an implementation to use a larger value than necessary for any of these fields (e.g. `is_large` may be true for an object with less than 256 elements).
-
-The "short string" basic type may be used as an optimization to fold string length into the type byte for strings less than 64 bytes. It is semantically identical to the "string" primitive type.
-
-The Decimal type contains a scale, but no precision. The implied precision of a decimal value is `floor(log_10(val)) + 1`.
-
-# Encoding types
-
-| Basic Type | ID | Description |
-|--------------|-----|---------------------------------------------------|
-| Primitive | `0` | One of the primitive types |
-| Short string | `1` | A string with a length less than 64 bytes |
-| Object | `2` | A collection of (string-key, variant-value) pairs |
-| Array | `3` | An ordered sequence of variant values |
-
-| Logical Type | Physical Type | Type ID | Equivalent Parquet Type | Binary format |
-|----------------------|-----------------------------|---------|-----------------------------|---------------------------------------------------------------------------------------------------------------------|
-| NullType | null | `0` | any | none |
-| Boolean | boolean (True) | `1` | BOOLEAN | none |
-| Boolean | boolean (False) | `2` | BOOLEAN | none |
-| Exact Numeric | int8 | `3` | INT(8, signed) | 1 byte |
-| Exact Numeric | int16 | `4` | INT(16, signed) | 2 byte little-endian |
-| Exact Numeric | int32 | `5` | INT(32, signed) | 4 byte little-endian |
-| Exact Numeric | int64 | `6` | INT(64, signed) | 8 byte little-endian |
-| Double | double | `7` | DOUBLE | IEEE little-endian |
-| Exact Numeric | decimal4 | `8` | DECIMAL(precision, scale) | 1 byte scale in range [0, 38], followed by little-endian unscaled value (see decimal table) |
-| Exact Numeric | decimal8 | `9` | DECIMAL(precision, scale) | 1 byte scale in range [0, 38], followed by little-endian unscaled value (see decimal table) |
-| Exact Numeric | decimal16 | `10` | DECIMAL(precision, scale) | 1 byte scale in range [0, 38], followed by little-endian unscaled value (see decimal table) |
-| Date | date | `11` | DATE | 4 byte little-endian |
-| Timestamp | timestamp | `12` | TIMESTAMP(true, MICROS) | 8-byte little-endian |
-| TimestampNTZ | timestamp without time zone | `13` | TIMESTAMP(false, MICROS) | 8-byte little-endian |
-| Float | float | `14` | FLOAT | IEEE little-endian |
-| Binary | binary | `15` | BINARY | 4 byte little-endian size, followed by bytes |
-| String | string | `16` | STRING | 4 byte little-endian size, followed by UTF-8 encoded bytes |
-
-| Decimal Precision | Decimal value type |
-|-----------------------|--------------------|
-| 1 <= precision <= 9 | int32 |
-| 10 <= precision <= 18 | int64 |
-| 18 <= precision <= 38 | int128 |
-| > 38 | Not supported |
-
-The *Logical Type* column indicates logical equivalence of physically encoded types. For example, a user expression operating on a string value containing "hello" should behave the same, whether it is encoded with the short string optimization, or long string encoding. Similarly, user expressions operating on an *int8* value of 1 should behave the same as a decimal16 with scale 2 and unscaled value 100.
-
-# Field ID order and uniqueness
-
-For objects, field IDs and offsets must be listed in the order of the corresponding field names, sorted lexicographically. Note that the fields themselves are not required to follow this order. As a result, offsets will not necessarily be listed in ascending order.
-
-An implementation may rely on this field ID order in searching for field names. E.g. a binary search on field IDs (combined with metadata lookups) may be used to find a field with a given field.
-
-Field names are case-sensitive. Field names are required to be unique for each object. It is an error for an object to contain two fields with the same name, whether or not they have distinct dictionary IDs.
-
-# Versions and extensions
-
-An implementation is not expected to parse a Variant value whose metadata version is higher than the version supported by the implementation. However, new types may be added to the specification without incrementing the version ID. In such a situation, an implementation should be able to read the rest of the Variant value if desired.
-
-# Shredding
-
-For columnar storage formats, a single Variant object may have poor read performance when only a small number of fields are needed. A better approach is to create separate columns for individual fields, referred to as shredding or subcolumnarization. [shredding.md](shredding.md) describes an approach to shredding Variant columns in Parquet and similar columnar formats.
+The Variant type specification has been moved to https://github.com/apache/parquet-format/blob/master/VariantEncoding.md and https://github.com/apache/parquet-format/blob/master/VariantShredding.md.
Review Comment:
To be honest, I'm not that familiar with the Apache Parquet release process. The last tag I see in github is from 2023. @gene-db, do you know if there's some other process, or has there been no official spec update since then?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51183][SQL] Link to Parquet spec in Variant docs [spark]
Posted by "gene-db (via GitHub)" <gi...@apache.org>.
gene-db commented on PR #49910:
URL: https://github.com/apache/spark/pull/49910#issuecomment-2654756302
What are the possible paths forward? We are currently in the transition period, so the spec is not fully finalized in parquet. Therefore, the options are:
- point to the (not finalized) Variant spec in Parquet (what this PR is doing)
- keep the Spark README as is (it is currently out-of-date)
- copy the text of the Variant spec in Parquet into the Spark README
We thought it would be best to link to the parquet docs, since it is going to be the source-of-truth moving forward, it reflects the current state of the implementation and will avoid confusion.
What do others think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51183][SQL] Link to Parquet spec in Variant docs [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #49910:
URL: https://github.com/apache/spark/pull/49910#issuecomment-2654655635
To @gene-db , please don't get me wrong. I'm aware of all the discussion completely. :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51183][SQL] Link to Parquet spec in Variant docs [spark]
Posted by "cashmand (via GitHub)" <gi...@apache.org>.
cashmand commented on PR #49910:
URL: https://github.com/apache/spark/pull/49910#issuecomment-2656729838
Hi @dongjoon-hyun, I updated the text to clarify the current status of the project. Let me know what you think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51183][SQL] Link to Parquet spec in Variant docs [spark]
Posted by "gene-db (via GitHub)" <gi...@apache.org>.
gene-db commented on PR #49910:
URL: https://github.com/apache/spark/pull/49910#issuecomment-2654602248
The Spark community had agreed to move the spec over to the parquet project, so we are currently in that process. It takes time for the spec to be formally finalized in Parquet, but it is definitely getting closer. The implementation in Spark is following the updated specs, and since the specs will be living in the parquet repo, it doesn't make sense to make a copy of the text in the Spark repo.
As for the parquet release process, once the specs are officially approved, the parquet community will do a release of the parquet-repo. While the specs are not officially approved, there are already several communities "unofficially" approving and implementing the Variant spec, so there is little risk of a breaking change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51183][SQL] Link to Parquet spec in Variant docs [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #49910:
URL: https://github.com/apache/spark/pull/49910#issuecomment-2654542782
cc @cloud-fan , @pan3793, @gene-db
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51183][SQL] Link to Parquet spec in Variant docs [spark]
Posted by "cashmand (via GitHub)" <gi...@apache.org>.
cashmand commented on PR #49910:
URL: https://github.com/apache/spark/pull/49910#issuecomment-2659975804
Is anything else required on this PR? If not, @dongjoon-hyun or @cloud-fan, can one of you please merge it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51183][SQL] Link to Parquet spec in Variant docs [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan closed pull request #49910: [SPARK-51183][SQL] Link to Parquet spec in Variant docs
URL: https://github.com/apache/spark/pull/49910
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51183][SQL] Link to Parquet spec in Variant docs [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #49910:
URL: https://github.com/apache/spark/pull/49910#issuecomment-2655509547
Yea it's just a dev doc as end-users shouldn't care about it. @dongjoon-hyun What's your concern about linking to the Parquet spec? As @pan3793 said we can explain the current status in the README and mention that the parquet spec is still WIP.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51183][SQL] Link to Parquet spec in Variant docs [spark]
Posted by "cashmand (via GitHub)" <gi...@apache.org>.
cashmand commented on PR #49910:
URL: https://github.com/apache/spark/pull/49910#issuecomment-2654575098
> I'm not sure this is a better idea or not because we have no tag-based link. In addition, the pointed document shows warnings like the following. This is a downgrade in terms of the documentation. <img alt="Screenshot 2025-02-12 at 10 42 21" width="742" src="https://private-user-images.githubusercontent.com/9700541/412566084-31927d48-0c02-49e1-8ce1-f1a4bc9f5783.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkzODYxNDYsIm5iZiI6MTczOTM4NTg0NiwicGF0aCI6Ii85NzAwNTQxLzQxMjU2NjA4NC0zMTkyN2Q0OC0wYzAyLTQ5ZTEtOGNlMS1mMWE0YmM5ZjU3ODMucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIxMiUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMTJUMTg0NDA2WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9ODgwOGUzYjIzMmIxMmZkYTNlMjQ2NDc2OGU3YmJjYjEwOTZlYzQ4Zjk4OWE3YTc5OWJiMDc4YjhmZTE0MTMyYiZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.T
kqxFgsZyHar2400GjRp1uQpIQYu5ZjA0TlbFptWcFQ">
>
> IMO, this doesn't look like a production-ready.
The spec is new to Parquet. We're making an effort to ensure that the spec is compatible with the implementation in Spark 4.0. I think the risk of a breaking change is very small, but maybe that's a motivation to include a similar warning in the Spark 4.0 release as well? I don't see much benefit to maintaining a second copy of the spec here in the meantime. @gene-db @cloud-fan, any opinion?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51183][SQL] Link to Parquet spec in Variant docs [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #49910:
URL: https://github.com/apache/spark/pull/49910#issuecomment-2655512345
Ya, if we are clear that this is WIP status, there is no problem. Eventually, I hope this is revised to point to the tagged version in the future.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51183][SQL] Link to Parquet spec in Variant docs [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #49910:
URL: https://github.com/apache/spark/pull/49910#issuecomment-2659979822
I'm leaving this PR to the release manager's final decision, @cloud-fan .
While waiting him, could you make the CI green although this is a doc-only PR, @cashmand ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51183][SQL] Link to Parquet spec in Variant docs [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #49910:
URL: https://github.com/apache/spark/pull/49910#issuecomment-2661946589
thanks, merging to master/4.0!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org