Skip to content

refactor(parquet): simplify row position handling in projections - #1020

Merged
wgtmac merged 1 commit into
apache:mainfrom
advancedxy:fix/parquet-nested-metadata-projection
Oct 16, 2026
Merged

wgtmac merged 1 commit into
apache:mainfrom
advancedxy:fix/parquet-nested-metadata-projection

Conversation

@advancedxy

Copy link
Copy Markdown
Contributor

What

Add a ProjectNestedArray overload for nested fields without a row-position argument, removing repeated nullptr arguments from recursive projection.

Use ICEBERG_PRECHECK at the record-batch and struct projection entries to validate required row positions. Add one regression test covering nested _pos and _row_id.

Why

Follow-up to the projection review in #1006.

The record-batch check only inspects top-level fields. Metadata fields nested in a struct could therefore reach projection with a null position array. Checking each struct before projection returns an error without adding checks to individual metadata branches.

Testing

  • parquet_test passed on the updated main baseline.
  • Pre-commit and git diff --check passed.

Limitations and follow-up

The current Iceberg reader supplies the separate position array only to top-level fields. This is an implementation limitation, not a restriction on schema projection. The planned Arrow C++ Parquet reader integration described in #1006 will expose positions in the reader's record batches.

@advancedxy
advancedxy force-pushed the fix/parquet-nested-metadata-projection branch from 06571f2 to ad0f270 Compare October 15, 2026 06:32
@advancedxy advancedxy changed the title fix(parquet): validate row positions in nested projections refactor(parquet): simplify row position handling in projections Oct 15, 2026
@wgtmac

wgtmac commented Oct 16, 2026

Copy link
Copy Markdown
Member

Thanks @advancedxy!

@wgtmac
wgtmac merged commit 310bb05 into apache:main Oct 16, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants