Skip to content

fix: preserve Parquet column key metadata associations - #26513

Draft
autoCAT6 wants to merge 1 commit into
apache:mainfrom
autoCAT6:fix-parquet-column-key-metadata
Draft

autoCAT6 wants to merge 1 commit into
apache:mainfrom
autoCAT6:fix-parquet-column-key-metadata

Conversation

@autoCAT6

@autoCAT6 autoCAT6 commented Oct 20, 2026 •

Copy link
Copy Markdown

Draft: Arrow-rs #11733 has merged. This still needs a Parquet release and dependency upgrade that provide FileEncryptionProperties::column_key_metadata; the current stock Parquet 60.0.0 dependency does not include it.

Which issue does this PR close?

No existing issue. The reproduction, root cause and regression coverage are described below.

Rationale for this change

Converting Parquet encryption properties to DataFusion configuration can move a key identifier to a different column when only some column keys have metadata. The key bytes retain their original column associations, so direct-key file reads may still succeed while metadata-driven key retrieval requests an incorrect identifier.

A mixed configuration can contain first with ID first-id and second with no ID. Depending on map iteration order, the converted configuration can instead put first-id on second and leave first without an ID.

The existing column_keys() metadata vector skips columns without metadata, whereas its names and keys vectors include every column. Using the same position to index these vectors therefore loses the name-to-metadata association.

What changes are included in this PR?

  • Retrieve each column's metadata by name using the upstream Parquet accessor.
  • Preserve absent metadata and explicitly empty metadata as distinct values.
  • Add public configuration roundtrip regressions and a real encrypted-file regression that inspects column crypto metadata and verifies exact values and nulls on readback.

The draft isolates the consumer change so it can be evaluated alongside the merged upstream API. It needs a dependency update once the API is released; the submitted diff contains no Cargo dependency override.

What is the testing strategy for this PR?

The regression cases cover mixed, all-present and all-absent column metadata, including explicitly empty metadata and footer metadata preservation. Independent property maps exercise unspecified map iteration order. Complete property equality checks the reverse conversion; actual file assertions verify the serialized column IDs rather than relying only on successful explicit-key reads.

Local validation uses Parquet 60.0.0 with only the proposed accessor added through a temporary Cargo patch. Results apply to that validation setup, not the current stock dependency:

With an isolated Parquet 60 source directory containing the accessor from the upstream PR, the focused regression command is:

cargo test -p datafusion-common --test encryption_metadata_roundtrip \
  --features parquet_encryption \
  --config 'patch.crates-io.parquet.path="/path/to/parquet-60-with-accessor"'
  • Four new regression tests pass; the original converter fails the mixed-metadata and file-metadata tests with the same accessor-only dependency.
  • datafusion-common: 664 unit tests, four new integration tests and 81 doctests pass; five existing doctests are ignored.
  • Formatting, modified-file spelling, strict all-target/all-feature workspace Clippy, and the required extended workspace suite pass with the temporary override; the extended suite includes all 535 SQL test files.

Are there any user-facing changes?

Encryption property conversion preserves each column's optional key metadata. No configuration fields or existing public signatures change.

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

Labels

common Related to common crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant