amogh-jahagirdar commented on code in PR #17320:
URL: https://github.com/apache/iceberg/pull/17320#discussion_r3979768925
##########
parquet/src/main/java/org/apache/iceberg/parquet/ParquetSchemaUtil.java:
##########
@@ -129,12 +131,94 @@ public static Type fieldType(GroupType group, String
name) {
public static MessageType pruneColumns(MessageType fileSchema, Schema
expectedSchema) {
Review Comment:
Agreed, and I looked into unifying this. The leaf selection is already
shared (both sides call selectPresenceColumn), but the "does this struct need a
presence column at all" decision is computed twice with different predicates:
- Prune side (PresenceColumnSelector.struct): checks whether any file leaf
under the struct's path is in the selected id set.
- Read side (ParquetValueReaders.presenceColumn): checks whether any direct
expected field has an entry in readersById.
The prune side reasons at leaf-id granularity across the whole subtree; the
read side reasons at direct-field granularity, because readersById is a local,
per-struct map built independently in each engine's reader builder (Spark x3,
Flink x4, the generic data module, Avro), not a running set threaded across the
whole schema traversal the way selectedIds is on the prune side. Making the
read side produce the same leaf-id set the prune side has would mean threading
a shared Set<Integer> through structReader's signature and every one of those
call sites, which is a lot of surface area for what's currently a latent, not
live, disagreement (a projected in-file leaf under a struct always yields a
field reader too today, so the two predicates agree in practice).
Given that, I'd rather not force it into this PR. Leaving it as a follow-up,
as you said - happy to pick it up separately.
--
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]
