sdf-jkl commented on PR #591: URL: https://github.com/apache/parquet-format/pull/591#issuecomment-4939397357
To add some clarity following the arrow-rs report (apache/arrow-rs#10306), here is a survey of how implementations currently handle a variant group that omits the `value` column: | implementation | reading a value-less group | writing | |---|---|---| | DuckDB | **rejects**: ["The Variant column must have 'metadata' and 'value' as the first two columns"](https://github.com/duckdb/duckdb/blob/985b2f2515adf44ea65dd4239c7bfb30981b94dc/extension/parquet/reader/variant_column_reader.cpp#L54-L62) | always emits `value` ([writer](https://github.com/duckdb/duckdb/blob/985b2f2515adf44ea65dd4239c7bfb30981b94dc/extension/parquet/writer/variant/convert_variant.cpp)) | | Spark | reads ([guards every `value` access](https://github.com/apache/spark/blob/c470f3dce544ad46ffb748949b6476d3cbc8d443/common/variant/src/main/java/org/apache/spark/types/variant/ShreddingUtils.java#L139-L168)) | always emits `value` ([`variantShreddingSchema` includes it at every level](https://github.com/apache/spark/blob/c470f3dce544ad46ffb748949b6476d3cbc8d443/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/SparkShreddingUtils.scala#L485-L512)) | | parquet-java | reads ([`containsField("value")` guards](https://github.com/apache/parquet-java/blob/ea349cd3e97538574604ee15248713f0c35dd0d9/parquet-variant/src/main/java/org/apache/parquet/variant/VariantConverters.java#L151)) | assumes `value` exists whenever a row needs the binary fallback ([`getFieldIndex` throws otherwise](https://github.com/apache/parquet-java/blob/ea349cd3e97538574604ee15248713f0c35dd0d9/parquet-variant/src/main/java/org/apache/parquet/variant/VariantValueWriter.java#L99-L108)) | | Arrow C++ | no shredding support yet; its unshredded storage validation [requires non-nullable `metadata` and `value`](https://github.com/apache/arrow/blob/5815dc0b4e3d9239828b47d427c9759fbc506e09/cpp/src/arrow/extension/parquet_variant.cc#L72-L105) | n/a | | arrow-go | reads; type validation currently requires only ["at least one of `value` or `typed_value`"](https://github.com/apache/arrow-go/blob/68e738f86506f61796e57d68551269552d76cdbf/arrow/extensions/variant.go#L176-L181) | may omit | | arrow-rs | currently reads (validation gap); fix in progress to reject (apache/arrow-rs#10306) | currently may omit `value`; fix in progress to always emit it (apache/arrow-rs#10306) | | cuDF | reads the raw struct without reconstructing values; [its tests assert the `value` child is present even for shredded columns](https://github.com/rapidsai/cudf/blob/0f98463d28c8346b2736c31420e4ef414228f5e2/python/cudf/cudf/tests/input_output/test_parquet_variant.py) (fixtures are DuckDB-written) | n/a | This supports the spec text as it stands: the implementations that validate variant group structure (DuckDB, Arrow C++) require `value`, arrow-rs is adopting the same, and every surveyed writer that produces spec-targeted files emits it. Where value-less groups are readable, it is by absence of validation rather than considered support. On how the divergence propagated: the `shredded_variant` cases in parquet-testing (generated from the Iceberg test suite in apache/parquet-testing#91) include four cases that omit `value` (41, 131, 132, 138) labeled as *valid* cases with expected outputs rather than with the `-INVALID` convention. arrow-rs and arrow-go both run that corpus in CI, so the lenient interpretation looked conformance-verified to both. I have filed https://github.com/apache/parquet-testing/issues/116 proposing those cases be relabeled per @alamb's suggestion above. With the spec question settled, the remaining work is enforcement in implementations: arrow-rs's is in progress under apache/arrow-rs#10306, and arrow-go's type validation currently accepts the omission and would need a similar update. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
