GH-50622: [Docs][Format] Align Variant typed_value primitive type mappings with the Parquet shredding spec - #50810
Conversation
|
|
|
Please take a look @alamb @wgtmac @zeroshade |
alamb
left a comment
There was a problem hiding this comment.
Thank you @sdf-jkl -- this looks very nice to me except for a few small details I left comments on
I don't think we should merge this without making sure we ping the mailing list about it (perhaps again) as I think it could be interpreted as changing the spec.
| only valid Parquet representation. | ||
|
|
||
| +----------------------------------------+--------------------------------------------------+---------------------------------------------+ | ||
| | Variant Type | Parquet Type | Arrow ``typed_value`` Type | |
There was a problem hiding this comment.
Perhaps we could also add a reference to the location of the parquet and arrow type lists
I think the arrow ones are here:
https://arrow.apache.org/docs/format/Columnar.html#data-types
The parquet ones are here:
https://parquet.apache.org/docs/file-format/types/
https://parquet.apache.org/docs/file-format/types/logicaltypes/
I am a little confused about what (logical or physical) type is used in the Parquet Type column. Some seem to be just the physical type (INT64) , and some seem to have both logical and physical (INT32, INT(8, true) ). It would help to be spcific about what the Parquet type column represents and use it consistently
| +----------------------------------------+--------------------------------------------------+---------------------------------------------+ | ||
| | double | DOUBLE | Float64 | | ||
| +----------------------------------------+--------------------------------------------------+---------------------------------------------+ | ||
| | decimal4 (1 <= P <= 9, 0 <= S <= P) | INT32, DECIMAL(P, S) | Decimal32(P, S) | |
There was a problem hiding this comment.
https://parquet.apache.org/docs/file-format/types/variantencoding/ says
The Decimal type contains a scale, but no precision. The implied precision of a decimal value is floor(log_10(val)) + 1.
I don't unersatnd how that maps to 1 <= P <= 9, 0 <= S <= P 🤔
There was a problem hiding this comment.
There's a small separate table for decimals that shows what precision can fit in the underlying physical type of a typed_value.
| | uuid | FIXED_LEN_BYTE_ARRAY[len=16], UUID | :ref:`UUID extension type <uuid_extension>` | | ||
| +----------------------------------------+--------------------------------------------------+---------------------------------------------+ | ||
|
|
||
| The decimal precision bands follow the `Variant encoding types |
There was a problem hiding this comment.
I am still a little confused about how decimals are working -- is the this text talking about each variant value itself (as the scale/precision can vary row to row) or the min/max precision of all values in a column (presumably what is needed when shredding) 😕
There was a problem hiding this comment.
It's talking about the constraint of physical types of the typed_value field, so it would be the latter. I'll polish it to make it more clear.
|
@alamb I think it's more clear now. |
| * A Variant null is always encoded in the ``value`` field (as ``00``), | ||
| never in ``typed_value``: a null ``typed_value`` signals that the row is | ||
| not shredded, and for shredded object fields a null ``typed_value`` | ||
| together with a null ``value`` means the field is missing. |
There was a problem hiding this comment.
Should we clarify that a variant null should still be indicated in the null validity bitmap of the corresponding array (accompanying the 00 value in the value field)?
There was a problem hiding this comment.
There was a problem hiding this comment.
I think it might be more precise to say it makes Variant::Null and a null value in the corresponding arrow array different things
| +----------------------------------------+-----------------------------------+--------------------------+---------------------------------------------+ | ||
| | int16 | INT32 | INT(16, true) | Int16 | | ||
| +----------------------------------------+-----------------------------------+--------------------------+---------------------------------------------+ | ||
| | int32 | INT32 | | Int32 | |
There was a problem hiding this comment.
i am confused that there is no logical type for int32, though this is the same in the parquet variant table too https://github.com/apache/parquet-format/blob/master/VariantShredding.md#shredded-value-types
| * A Variant null is always encoded in the ``value`` field (as ``00``), | ||
| never in ``typed_value``: a null ``typed_value`` signals that the row is | ||
| not shredded, and for shredded object fields a null ``typed_value`` | ||
| together with a null ``value`` means the field is missing. |
There was a problem hiding this comment.
I think it might be more precise to say it makes Variant::Null and a null value in the corresponding arrow array different things
Rationale for this change
The existing Arrow spec is misleading and allows certain Arrow types to be typed value Variant fields. The Parquet Shredding spec specifically rejects them - Unsigned integers, Null.
Decimal mapping needs clarification about precision.
What changes are included in this PR?
Change the Primitive Type Mappings table for Parquet Variant extension type.
Rendered version - https://github.com/sdf-jkl/arrow/blob/gh50622-variant-typed-value-mappings/docs/source/format/CanonicalExtensions.rst#primitive-type-mappings
Are these changes tested?
N/A
Are there any user-facing changes?
Changes to the Arrow docs
Closes #50622
typed_valueprimitive type mappings with the Parquet shredding spec #50622