github-actions[bot] commented on code in PR #67465:
URL: https://github.com/apache/doris/pull/67465#discussion_r3920986736
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -1759,6 +1757,27 @@ public TableScan createTableScan() throws UserException {
return icebergTableScan;
}
+ @VisibleForTesting
+ protected Expression convertToIcebergPruningExpression(Expr conjunct,
Schema scanSchema) {
+ Expression scanExpression =
IcebergUtils.convertToIcebergExpr(conjunct, scanSchema);
+ Schema currentSchema = icebergTable.schema();
+ if (scanExpression == null || scanSchema.sameSchema(currentSchema)) {
+ return scanExpression;
+ }
+
+ Expression currentExpression =
IcebergUtils.convertToIcebergExpr(conjunct, currentSchema);
+ if (currentExpression == null) {
+ return null;
+ }
+ Set<Integer> scanFieldIds = Binder.boundReferences(
+ scanSchema.asStruct(),
Collections.singletonList(scanExpression), true);
+ Set<Integer> currentFieldIds = Binder.boundReferences(
+ currentSchema.asStruct(),
Collections.singletonList(currentExpression), true);
+ // Iceberg 1.10 plans manifests with current partition specs, so only
push an
+ // historical predicate when its names still resolve to the same field
IDs.
+ return scanFieldIds.equals(currentFieldIds) ? scanExpression : null;
Review Comment:
[P1] Validate each predicate leaf, not the aggregate ID set
`boundReferences` returns a set, so equality here does not prove that the
returned unbound expression has the same binding under both schemas. For
example, historical `1:a, 2:b` can legally evolve via `a -> tmp`, `b -> a`,
`tmp -> b` to current `1:b, 2:a`; `a = 1 OR b = 2` yields `{1,2}` on both
sides, passes this guard, and Iceberg 1.10.1 then rebinds the historical names
against current partition specs, allowing a matching historical file to be
pruned. A case-only rename (`Foo` -> `foo`) also passes `{7} == {7}`, but the
returned `Foo` fails case-sensitive binding against the current spec. Please
verify each converted leaf's name-to-ID correspondence (including exact
spelling), or suppress pushdown, and add partitioned/case-only regressions that
actually attach the predicate to `planFiles()`.
--
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]
