Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/22995 )
Change subject: POC IMPALA-14123: Allow forcing predicate push down to Iceberg ...................................................................... Patch Set 2: (4 comments) Thanks for working on this, I think the overall approach is good. http://gerrit.cloudera.org:8080/#/c/22995/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22995/2//COMMIT_MSG@25 PS2, Line 25: headers nit: footers/file metadata http://gerrit.cloudera.org:8080/#/c/22995/2/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java: http://gerrit.cloudera.org:8080/#/c/22995/2/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@181 PS2, Line 181: String HINT_KEY = "impala.iceberg.push_down_hint"; : String hint = : tblRef_.getDesc().getTable().getMetaStoreTable().getParameters().get(HINT_KEY); : if (hint != null) { : for (String col: hint.split(",")) { : columnsWithPushDownHint_.add(col.toLowerCase()); : } : } nit: this could be moved to its own method http://gerrit.cloudera.org:8080/#/c/22995/2/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@945 PS2, Line 945: Compuoun nit: Compound http://gerrit.cloudera.org:8080/#/c/22995/2/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@943 PS2, Line 943: // TODO: the semantics of the function are not clear - shouldn't : // it only return true if ALL cols in the expr have the given : // partition type? Compuoun predicates with OR can have multiple : // columns. Not sure what is the best semantics here, but we should treat partitions/hint columns similarly. Also, there can be mixed cases: a = 3 OR b = 4 'a' can be a partition column, while 'b' can be a hint column. I think we should go with the current logic for now, to not cause any regressions, also it is quite simple. -- To view, visit http://gerrit.cloudera.org:8080/22995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8eb4ab5204c20b3991fdf305d7317f4023904a0f Gerrit-Change-Number: 22995 Gerrit-PatchSet: 2 Gerrit-Owner: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Fri, 20 Jun 2025 14:32:22 +0000 Gerrit-HasComments: Yes
