qinghui-xu commented on code in PR #16280:
URL: https://github.com/apache/iceberg/pull/16280#discussion_r3387769512
##########
data/src/main/java/org/apache/iceberg/data/DeleteFilter.java:
##########
@@ -73,13 +72,19 @@ protected DeleteFilter(
Schema expectedSchema,
DeleteCounter counter,
boolean needRowPosCol) {
- this(filePath, deletes, tableSchema::findField, expectedSchema, counter,
needRowPosCol);
+ this(
+ filePath,
+ deletes,
+ ids -> TypeUtil.project(tableSchema, ids),
+ expectedSchema,
+ counter,
+ needRowPosCol);
}
protected DeleteFilter(
String filePath,
List<DeleteFile> deletes,
- Function<Integer, Types.NestedField> fieldLookup,
+ Function<Set<Integer>, Schema> missingSchemaResolver,
Review Comment:
Hello @pvary
> Instead of this change, shouldn't we just introduce our own method for
fieldLookup?
I was thinking you agreed with my comment on this point, so I did not
address it earlier, or maybe there's some confusion. My suggestion is to
introduce the method for "fieldLookup" in the `DeleteFilter` to implement the
projection logic in order to correctly build the `requiredSchema` with all the
nested fields. And such method can be overridden if needed in subclasses, such
as in spark v4.1 implementation.
The "function" argument is a bit cumbersome for the constructor and I had a
hard time to think of a clear & short naming without leading to any confusion.
And such function itself is an indication of some behavior customization, so a
method override in subclasses is more appropriate in this case.
> Schema.findField only returns the leaf column NestedField object, but
instead we could return the whole hierarchy. Do we need any other change in
this case?
I think that's what the current PR is doing?
--
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]
