peterxcli opened a new issue, #5477:
URL: https://github.com/apache/datafusion-comet/issues/5477
### What is the problem the feature request solves?
Native Variant projection in #5407 includes a few compatibility paths that
exist because Comet currently uses Arrow/Parquet 58.4.0 or must interoperate
with older Spark Variant encodings. These paths are intentionally narrow, but
some perform row-by-row decoding or rebuilding and should not become permanent
accidentally.
This issue records exactly which code is removable after an upstream fix
ships, which upstream change owns it, and which nearby code is required
Comet/Spark integration and must remain.
This is maintenance follow-up for #5407 under the Native Variant epic #5438.
Spark UTF-16 output cleanup is tracked separately by #5474.
#### Arrow-rs workarounds
- [ ] **Dictionary-encoded Variant metadata**
- Comet eagerly decodes Dictionary metadata in
[`decode_variant_metadata_dictionary`](https://github.com/apache/datafusion-comet/blob/ba37a688e5aaa1d9e4a17c55d42ea0211486f7a3/native/core/src/parquet/cast_column/variant.rs#L356-L392),
called [before
`VariantArray::try_new`](https://github.com/apache/datafusion-comet/blob/ba37a688e5aaa1d9e4a17c55d42ea0211486f7a3/native/core/src/parquet/cast_column/variant.rs#L59-L61).
- Upstream: apache/arrow-rs#10802 and apache/arrow-rs#10810.
- Remove the call and helper after Comet upgrades to an Arrow-rs release
whose `VariantArray` and downstream compute support Dictionary and
RunEndEncoded metadata. Keep the focused regression test so it then exercises
Arrow-rs directly.
- [ ] **Empty Variant object keys**
- Arrow-rs 58.4 rejects equal metadata offsets used by empty dictionary
entries. Comet retries unshredding in
[`unshred_variant_for_spark`](https://github.com/apache/datafusion-comet/blob/ba37a688e5aaa1d9e4a17c55d42ea0211486f7a3/native/core/src/parquet/cast_column/variant.rs#L89-L101),
canonicalizes affected rows in
[`canonicalize_spark_empty_key_metadata`](https://github.com/apache/datafusion-comet/blob/ba37a688e5aaa1d9e4a17c55d42ea0211486f7a3/native/core/src/parquet/cast_column/variant.rs#L227-L354),
and uses shallow metadata parsing at [the source-metadata
rebuild](https://github.com/apache/datafusion-comet/blob/ba37a688e5aaa1d9e4a17c55d42ea0211486f7a3/native/core/src/parquet/cast_column/variant.rs#L996-L1000)
and [value
reordering](https://github.com/apache/datafusion-comet/blob/ba37a688e5aaa1d9e4a17c55d42ea0211486f7a3/native/core/src/parquet/cast_column/variant.rs#L1088-L1095).
- Upstream: apache/arrow-rs#10352, merged but not present in Comet's
current dependency.
- After upgrading to a release containing the fix, remove the retry and
canonicalizer, restore `VariantMetadata::try_new` at the source-consumption
sites, and change tests that assert the old rejection to assert successful
validation.
- Here, “empty field name” means an empty Variant object key such as `{"":
1}`, not an empty Arrow schema field name.
- [ ] **Unsigned shredded `typed_value` fields**
- Comet widens only [`UInt8 -> Int16`, `UInt16 -> Int32`, and `UInt32 ->
Int64`](https://github.com/apache/datafusion-comet/blob/ba37a688e5aaa1d9e4a17c55d42ea0211486f7a3/native/core/src/parquet/cast_column/variant.rs#L109-L112)
before unshredding.
- Upstream options are unresolved: apache/arrow-rs#10416 /
apache/arrow-rs#10417 add widening, while apache/arrow#50622 /
apache/arrow#50810 propose removing unsigned mappings from the canonical
Variant shredding table.
- Remove the three unsigned arms if Arrow-rs provides the widening in a
released dependency. If the canonical format rejects unsigned shredding
instead, retain them only as an explicit legacy/noncanonical-file compatibility
policy or fall back safely. Do not remove the shared recursive normalizer
wholesale.
### Describe the potential solution
For each checklist item:
1. Wait for the relevant upstream behavior to merge and appear in a released
dependency.
2. Upgrade Arrow/Parquet through the normal dependency update.
3. Delete only the mapped compatibility branch; do not duplicate the
upstream implementation locally.
4. Keep or adapt the existing regression tests to prove the upstream path
works through Comet.
5. Run the focused Rust Variant tests, Spark 4 Variant SQL tests, Spark 3
compilation boundary, formatting/lint, and `git diff --check`.
Definition of done:
- Every temporary upstream workaround is either removed or explicitly
retained with a current compatibility reason.
- Dictionary and RunEndEncoded metadata, empty object keys, and the chosen
unsigned-input policy remain covered.
- Whole-value Variant projection still returns Spark's required `[value,
metadata]` layout and preserves SQL NULL versus Variant JSON null.
- No Spark/FFI integration code is removed merely because an Arrow-rs
dependency was upgraded.
### Additional context
The following nearby code is **not** covered by the Arrow-rs fixes above:
- Millisecond timestamp and `FixedSizeList -> List` normalization are
Spark-reader compatibility for physical types outside the canonical Arrow
Variant mapping. apache/arrow-rs#10417 does not cover them.
-
[`prepare_variant_for_unshredding`](https://github.com/apache/datafusion-comet/blob/ba37a688e5aaa1d9e4a17c55d42ea0211486f7a3/native/core/src/parquet/cast_column/variant.rs#L190-L225),
`SparkMetadataBuilder`, and the [Spark-format
reconstruction](https://github.com/apache/datafusion-comet/blob/ba37a688e5aaa1d9e4a17c55d42ea0211486f7a3/native/core/src/parquet/cast_column/variant.rs#L967-L1119)
handle canonical UTF-8 versus legacy Spark UTF-16 ordering and Spark
byte-format compatibility. Their output-side simplification belongs to #5474;
an input-side path may remain for historical Spark-written files.
-
[`ffi_schema_for_field`](https://github.com/apache/datafusion-comet/blob/ba37a688e5aaa1d9e4a17c55d42ea0211486f7a3/native/core/src/execution/utils.rs#L27-L41)
substitutes embedded NUL because the Arrow C Data Interface uses
NUL-terminated names. That is an ABI limitation, unrelated to
apache/arrow-rs#10352.
- [Exporting the complete Arrow
`Field`](https://github.com/apache/datafusion-comet/blob/ba37a688e5aaa1d9e4a17c55d42ea0211486f7a3/native/core/src/execution/utils.rs#L43-L84),
preserving `ARROW:extension:name=arrow.parquet.variant`, unshredding to
`[value, metadata]`, materializing ordinary Binary children for Spark,
preserving parent nulls, and retaining explicit fallback gates are required
Comet integration.
Current dependency: [Arrow/Parquet
58.4.0](https://github.com/apache/datafusion-comet/blob/ba37a688e5aaa1d9e4a17c55d42ea0211486f7a3/native/Cargo.toml#L41-L45).
--
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]
