Skip to content

feat: adapt Parquet storage for Variant projection - #5794

Open
peterxcli wants to merge 1 commit into
apache:mainfrom
peterxcli:codex/variant-parquet-compatibility
Open

feat: adapt Parquet storage for Variant projection#5794
peterxcli wants to merge 1 commit into
apache:mainfrom
peterxcli:codex/variant-parquet-compatibility

Conversation

@peterxcli

@peterxcli peterxcli commented Sep 9, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

Closes #5550. Parent: #5546.

Fourth split of #5546

Rationale for this change

Spark and Arrow can interpret the same Parquet storage differently. When reconstructing a projected Variant, these differences can change a value, reject a file Spark accepts, or select the wrong column.

What changes are included in this PR?

Use physical Parquet types for scans that project a marked Variant column, preserving ENUM as a string. Rebuild the returned footer metadata while retaining row groups, page indexes, and the original cached metadata.

Normalize encoded children, unsigned integers, supported wide decimals, millisecond timestamps, fixed binary, and fixed lists before Variant reconstruction. Preserve UUID rejection, report conversion overflow, enforce field ID precedence, and reject encrypted Variant scans at the native boundary.

JVM scan admission and Spark integration acceptance are tracked in #5551.

How are these changes tested?

Generated Parquet inputs cover physical type interpretation, ENUM/string/binary distinctions, wide decimals, schema hints, and unread Variant pruning. Native tests also cover storage normalization, overflow, field ID shadowing, encrypted scan rejection, and footer metadata preservation.

Validation from native/:

  • cargo test -p datafusion-comet --lib parquet::: 129 passed, 1 ignored benchmark.
  • cargo clippy -p datafusion-comet --lib --tests -- -D warnings: passed.
  • cargo fmt --all -- --check: passed.

git diff --check passed. Spark integration tests were not run.

@github-actions github-actions Bot added enhancement New feature or request area:scan Parquet scan / data reading labels Sep 9, 2026

@sunchao sunchao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness

Reviewed edf1d422 against 5627ab8c. This supplies the physical-reader compatibility layer for whole Variant reconstruction. Previously, advisory ARROW:schema hints could restore an Arrow representation that differs from Spark's interpretation of the file, ENUM could become binary, and otherwise supported unsigned, decimal or fixed-width children could be rejected or reconstructed as a different Variant type. The change applies the footer policy only when the required schema projects an explicitly marked top-level Variant, then normalizes storage before the existing reconstruction path. I found no P1/P2 issue in this split.

The conversions agree with the maintained Spark 4.0 ParquetToSparkSchemaConverter and SparkShreddingUtils: unsigned widths widen without losing their range, UINT64 becomes Decimal(20,0), ENUM remains a string, and unannotated fixed binary remains binary. The precision-limited Decimal256 conversion covers Arrow's choice of storage for wide physical decimals, while checked millisecond conversion reports overflow. Enabling the canonical UUID annotation lets the normalizer reject actual UUID storage before converting unannotated fixed binary. Existing INT96 microsecond/UTC coercion still runs after footer inference, so removing the original Arrow hint does not remove that policy.

The field-ID correction also matches Spark 3.5/4.0 matchIdField: an ID-bearing target cannot fall back to a different column with the same name. Resolving the physical column's own ID first, then hiding a shadowing name with collision-checked synthetic names, preserves that precedence and the existing shared Unicode matching. Footer reconstruction retains row groups, column orders and both page indexes without mutating the cached original. The initialization and reader guards reject encrypted projected Variant before losing private decryption state.

The current Linux Rust job passed all 1,268 executed tests, including all ten new tests and actual generated-Parquet scans. CI checked out e021943f, whose parents are the assigned base and reviewed head and whose complete tree equals the head tree. The Spark 3.4.3 and Spark 4.2.0 scan jobs passed general scan regressions. The Spark 3.5.9 scan job failed before compilation/tests because Maven Central returned HTTP 502 for sslext:1.2-0. At 2026-09-09 14:09 UTC, checks were 38 successful, 21 running, one failed, one queued and seven skipped. The remaining CI is still pending.

This is native-layer validation. The JVM Variant gate remains closed as required by #5550, and #5551 owns admission and Spark integration acceptance. No local runtime test was run, and the missing maintained Spark 3.4/4.1 branches prevent canonical-source claims for those versions.

Performance

The additional work is scoped to affected scans and representations. An unread Variant does not activate footer rewriting, metadata needing no rewrite retains its original Arc, and arrays needing no type change reuse the input. A rewritten footer does copy row-group and page-index containers, and affected batches pay for recursive schema inspection plus required decoding/casts before reconstruction. Those costs are real, especially for large indexed footers. No benchmark or speedup is claimed here, and I did not measure them. The shared field-ID change uses pre-folded names and initializes the collision set only when needed, avoiding additional per-pair JVM calls.

For the admission work tracked in #5551, could you include focused microbenchmarks for large indexed footers and plain versus encoded/nested Variant batches? The type and UUID decisions depend on the physical schema, but the current path walks that schema twice per batch and builds temporary field vectors even when no cast is needed. Those measurements should determine whether to cache the normalization decision per physical schema before enabling the path. This review does not establish a measured regression.

Design

Keeping physical schema interpretation before array normalization preserves information that Arrow would otherwise discard, particularly the ENUM/binary and UUID/unannotated-binary distinctions. The existing reconstruction code then continues to own Spark's output layout and Variant encoding rules. Rebuilding only the returned footer preserves the shared cache's meaning for other scans. The explicit encryption rejection is consistent with the narrower admission planned in #5551. The removal conditions in #5477 give these compatibility paths a concrete upstream cleanup boundary.

Abstraction & complexity

The helpers stay within the existing reader factory, schema adapter and Variant normalizer. The factory's default-disabled policy makes the changed footer behavior explicit, and one recursive type normalizer covers encoded and nested children without duplicating the Variant reconstruction machinery. The field-ID repair belongs in the shared adapter because its rule applies to ordinary columns too. I have no additional abstraction or restructuring to request within this scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:scan Parquet scan / data reading enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Variant] Match Spark physical Parquet semantics for projected Variant columns

2 participants