apache / apache/datafusion-comet
[Variant] Track and remove native Variant compatibility workarounds after upstream fixes
- Dominant language
- Scala
- Stars
- 1.3k
- Forks
- 373
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 190
Description
### What is the problem the feature request solves?
Native Variant projection from #5407, continued by #5868, includes compatibility paths for gaps in upstream Variant compute, Spark-compatible Parquet reader policy, and older Spark Variant encodings. As of 2026-09-13, Comet `main` locks **DataFusion 55.1.0 and Arrow/Parquet 59.3.0**; #5868 currently locks DataFusion 55.0.0 and Arrow/Parquet 59.3.0. The dependency upgrade is no longer a blocker. These paths are intentionally narrow, but some perform row-by-row decoding or metadata 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. Unicode case-insensitive Parquet field matching (#5495) is complete through #5602. The split series in #5546 reuses that shared matcher and does not carry the original PR's Unicode guards.
#### Arrow-rs compute workarounds
- [x] **Encoded Variant metadata**
- Comet recursively decodes Dictionary arrays before `VariantArray::try_new` in [`normalize_variant_storage`](https://github.com/apache/datafusion-comet/blob/6e556c944873554310aedd6a27f0d3e77ef17e71/native/core/src/parquet/cast_column/variant.rs).
- Upstream apache/arrow-rs#10802 and apache/arrow-rs#10810 cover Dictionary and RunEndEncoded canonical `metadata` only.
- After upgrading to a release containing that fix, stop decoding `metadata` locally and keep focused regressions exercising upstream directly. This does not remove compatibility for other encoded children.
- [ ] **Dictionary-encoded `value` and `typed_value`**
- Dedicated compatibility feature request: apache/arrow-rs#11068. Current Arrow-rs main rejects both representations in `VariantArray::try_new`; #10810 does not cover them.
- The canonical Arrow Variant extension explicitly permits encodings for `metadata`, not these children. This is a compatibility-policy decision, not an unfixed canonical-input bug.
- Embedded Arrow schemas can restore dictionary types. Retain the recursive decoder until upstream provides supported normalization, or Comet proves reader-side physical inference/fallback excludes such inputs at every supported entry point.
- [x] **Empty Variant object keys**
- Arrow-rs 59.2.0's [full metadata validator still requires strictly increasing offsets for unsorted dictionaries](https://github.com/apache/arrow-rs/blob/59.2.0/parquet-variant/src/variant/metadata.rs#L311-L321), rejecting equal offsets used by empty dictionary entries. Comet retries unshredding in [`unshred_variant_for_spark`](https://github.com/apache/datafusion-comet/blob/23bfd83b8ffc1071b487a90d8dff26019bb2f1bf/native/core/src/parquet/cast_column/variant.rs#L91-L112), canonicalizes affected rows in [`canonicalize_spark_empty_key_metadata`](https://github.com/apache/datafusion-comet/blob/23bfd83b8ffc1071b487a90d8dff26019bb2f1bf/native/core/src/parquet/cast_column/variant.rs#L584-L651), and uses shallow metadata parsing while rebuilding values.
- 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, use fallible metadata construction followed by `with_full_validation` at 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 [`UInt8 -> Int16`, `UInt16 -> Int32`, and `UInt32 -> Int64`](https://github.com/apache/datafusion-comet/blob/23bfd83b8ffc1071b487a90d8dff26019bb2f1bf/native/core/src/parquet/cast_column/variant.rs#L121-L130) before unshredding. `UInt64 -> Decimal(20,0)` remains Spark-specific.
- 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 first 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.
- [ ] **Wide physical DECIMAL `typed_value` fields**
- Current Arrow-rs main still infers `FIXED_LEN_BYTE_ARRAY` wider than 16 bytes as Decimal256 even when the declared precision is at most 38. `VariantArray` rejects Decimal256, while Spark reads the declared `DecimalType` in its 128-bit representation.
- Comet recursively narrows [`Decimal256(p, s) -> Decimal128(p, s)` for `p <= 38`](https://github.com/apache/datafusion-comet/blob/22f588aa67f3a9b893cccf8e64010a0d41c14360/native/core/src/parquet/cast_column/variant.rs#L121-L135). Runtime regressions cover positive and negative DECIMAL(38,2) values stored in 17- and 32-byte physical fields.
- Dedicated upstream bug: apache/arrow-rs#11067, reproduced on Arrow-rs `4cd8be954f6bc6b6dd265140207365b59a9900ec` for both physical widths and signs. This is not fixed by stripping `ARROW:schema` or by https://github.com/apache/arrow-rs/pull/10417. Remove the arm only if upstream Variant compute accepts the representation or the reader can supply Spark-compatible physical inference without it.
- [ ] **Shredded object keys absent from metadata**
- #5868 adds [`extend_shredded_metadata`](https://github.com/apache/datafusion-comet/blob/6e556c944873554310aedd6a27f0d3e77ef17e71/native/core/src/parquet/cast_column/variant.rs#L586-L658): add missing keys from the shredded schema and remap residual field IDs before unshredding.
- Dedicated upstream panic bug: apache/arrow-rs#11069. Current Arrow-rs main panics when a present shredded field name is missing from the row's metadata.
- The current [Parquet specification](https://github.com/apache/parquet-format/blob/master/VariantShredding.md#variant-metadata) requires all shredded field names in metadata. Comet's repair preserves permissive Spark compatibility for inputs outside that requirement.
- An upstream error instead of a panic is insufficient to remove this repair. Remove it only if upstream supports equivalent permissive reconstruction, or Comet deliberately changes this compatibility policy to a safe fallback. Preserve residual ID remapping, nested objects/lists, absent fields, SQL NULL and Variant null coverage.
#### DataFusion/Arrow reader integration workaround
- [ ] **Use Spark-compatible physical schema inference for projected Variant scans**
- Spark ignores the advisory `ARROW:schema` footer hint, while Arrow-rs honors it and can restore shredded leaves as `Decimal256`, `Date64`, Dictionary, or fixed-size types. Comet therefore replaces an embedded hint with physical Parquet inference for an unencrypted scan whose pruned required schema projects Variant.
- Arrow-rs also maps physical Parquet ENUM to Binary, while Spark maps ENUM to String. Once Arrow inference has discarded that annotation, the Variant normalizer cannot distinguish ENUM from raw BINARY. Comet therefore adds back a physical-schema-derived hint that changes only ENUM leaves to Utf8 ([ENUM mapping](https://github.com/apache/datafusion-comet/blob/22f588aa67f3a9b893cccf8e64010a0d41c14360/native/core/src/parquet/eager_page_index_reader_factory.rs#L147-L227), [footer rewrite](https://github.com/apache/datafusion-comet/blob/22f588aa67f3a9b893cccf8e64010a0d41c14360/native/core/src/parquet/eager_page_index_reader_factory.rs#L229-L271)). Runtime controls verify ENUM and STRING become Variant strings while unannotated BINARY remains binary.
- All other key/value metadata, row groups, column orders, column indexes, and offset indexes are preserved. Using the pruned schema also preserves #5377: an unread Variant column does not activate this path. Encrypted Variant scans remain on Spark because reconstructing public `FileMetaData` cannot preserve arrow-rs's private decryptor state.
- Reassessment: merged apache/datafusion#22360 already supplies `PartitionedFile.arrow_schema`. Arrow's `ArrowReaderOptions::with_schema` suppresses advisory Arrow metadata and accepts nested Binary-to-Utf8 hints. A local check verified ENUM-to-Utf8, raw BINARY preservation, bypass of an invalid Arrow hint, and reuse of the original metadata Arc. No additional Arrow ENUM-conversion API is needed for a known full per-file schema.
- Remaining upstream integration request: apache/datafusion#25251. Comet learns each file's shredded layout during lazy footer loading; the current reader factory cannot return a newly derived schema/options to DataFusion's opener. Allow source-specific physical-schema policy after footer loading and before Arrow schema inference, preserving the original metadata, cache behavior and decryption state. #22360 is sufficient when that complete schema is already available before opening.
- apache/datafusion#25192 covers nested table-schema coercions; it does not supply missing per-file shredded layouts or physical ENUM annotations.
- Remove the footer reconstruction when the existing supplied-schema route can be used at Comet's lazy-open boundary. Reassess the encrypted fallback only after encryption-specific validation; preserving the metadata Arc alone is not an encrypted-scan test.
### Describe the potential solution
For each checklist item:
1. Wait for the relevant upstream behavior to merge and appear in a released dependency or reachable DataFusion reader API.
2. Upgrade Arrow/Parquet/DataFusion through the normal dependency update.
3. Delete only the mapped compatibility branch; do not duplicate an 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.
- Encoded metadata, the chosen encoded-child policy, empty object keys, missing shredded-key compatibility, wide decimals, the chosen unsigned-input policy, and physical-schema inference remain covered.
- Whole-value Variant projection still returns Spark's required `[value, metadata]` layout and preserves SQL NULL versus Variant JSON null.
- Unread Variant pruning remains native, and encrypted scans do not lose encryption state.
- 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 upstream fixes above:
- Millisecond timestamp, unannotated fixed-length binary, `UInt64 -> Decimal(20,0)`, 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/23bfd83b8ffc1071b487a90d8dff26019bb2f1bf/native/core/src/parquet/cast_column/variant.rs#L541-L582), [`SparkMetadataBuilder`](https://github.com/apache/datafusion-comet/blob/23bfd83b8ffc1071b487a90d8dff26019bb2f1bf/native/core/src/parquet/cast_column/variant.rs#L654-L702), and the [Spark-format reconstruction](https://github.com/apache/datafusion-comet/blob/23bfd83b8ffc1071b487a90d8dff26019bb2f1bf/native/core/src/parquet/cast_column/variant.rs#L1269-L1408) 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/23bfd83b8ffc1071b487a90d8dff26019bb2f1bf/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/23bfd83b8ffc1071b487a90d8dff26019bb2f1bf/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.
- Spark-compatible case-insensitive Parquet column matching is implemented in the shared schema adapter by #5602 (closes #5495). The remaining Variant splits reuse it without the original PR's non-ASCII fallback or fail-closed guard. This does not remove #5474's encoded object-key ordering compatibility or the separate field-ID shadowing work in #5550.
Dependency snapshot (2026-09-13): [Comet main locks DataFusion 55.1.0 and Arrow/Parquet 59.3.0](https://github.com/apache/datafusion-comet/blob/0690d38d3cc8f634dffb5341783f4b5da180eceb/native/Cargo.lock); [#5868 locks DataFusion 55.0.0 and Arrow/Parquet 59.3.0](https://github.com/apache/datafusion-comet/blob/6e556c944873554310aedd6a27f0d3e77ef17e71/native/Cargo.lock). Links to the original #5407 implementation describe the compatibility branches to reassess, not the current main dependency. Checked upstream-fix entries do not by themselves mean Comet's released dependency contains the fix.
Contributor guide
Research direction
Start with native/core/src/parquet/cast_column/variant.rs and native/core/src/parquet/eager_page_index_reader_factory.rs, then review the linked upstream fixes and DataFusion reader APIs. Run the focused Rust Variant tests and Spark 4 Variant SQL tests before changing anything. Done means each workaround is removed only when its upstream replacement is released and all listed compatibility, pruning, null, and encryption regressions remain covered.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend, data-engineering
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100