jzhuge opened a new pull request, #18336: URL: https://github.com/apache/iceberg/pull/18336
## What `PruneColumns.struct()` adds the original, unpruned Parquet field whenever the field's id is in `selectedIds`. A parent struct's id is in `selectedIds` whenever *any* descendant is projected, so reading one leaf of a deeply nested struct widens the Parquet read schema back to the full intermediate struct and everything under it. This change adds the visitor's pruned field instead, and accumulates `hasChange` (`|=`) so a wholly projected sibling visited later cannot reset pruning recorded by an earlier sibling at the same level. ## Why Reading `l1.l2.l3.leaf` from `l1 struct<l2 struct<l3 struct<leaf bigint, b0..b19 string>>>` read all of `l2`. On a 16.8 GB Parquet table (1M rows, warm cache, executor task time summed over the scan stage): - id-only control: ~27-30k ms - deepest leaf only: ~223-227k ms (~8x control), the same as projecting half of `l2` - whole `l2`: ~274-278k ms Reproduced on Spark 3.5 and Spark 4.1 runtimes, and without Spark through `ParquetSchemaUtil.pruneColumns`. Whole-struct and top-level projections produce the same read schema as before. ## Design notes 1. **Select-struct-by-id is preserved.** Projections expand a selected struct to its full subtree, so a whole-struct selection gives `field == originalField` at every level and keeps the old path. Only path-parent structs with partially projected descendants change. 2. **`hasChange` must accumulate.** Overwriting it per sibling (`hasChange = !equal(...)`) makes pruning order-dependent: a fully projected later sibling resets the flag and re-widens an earlier partially projected sibling whenever no field was dropped at that level. Covered by `testDeeplyNestedStructPartiallyProjectedBeforeFullyProjected`. 3. **`list()` and `map()` are unchanged.** Once `struct()` returns the pruned field, structs inside list elements and map values prune correctly with no container change (see the list/map tests). 4. **Fields without ids keep the pruned subtree.** The `field != null` branch for unselected or id-less fields now adds the pruned result instead of the original. This only affects files with partial field-id assignment. ## History Revives two stale-closed attempts, neither rejected on merit: #12634 (struct-only variant of this fix) and #14744 (broader list/map changes). Fixes #11332 (closed as stale, not fixed). ## Verification - `TestPruneColumns`: 6 new tests. 4 fail on `main` and pass with the fix: `testDeeplyNestedStructProjection`, `testDeeplyNestedStructMixed`, `testDeeplyNestedStructInsideList`, `testDeeplyNestedStructPartiallyProjectedBeforeFullyProjected`. 2 pass both before and after and pin unchanged behavior: `testDeeplyNestedStructWhole`, `testDeeplyNestedStructInsideMap`. - `./gradlew :iceberg-parquet:check` passes with the patch. --- Prepared by Aimee (an AI agent) on behalf of John Zhuge, from the fix by Ruijing Li. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
