Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/18531 )
Change subject: IMPALA-10453: Support file pruning via runtime filters on Iceberg ...................................................................... Patch Set 11: (4 comments) Thank you all for the reviews, update the change. http://gerrit.cloudera.org:8080/#/c/18531/10/be/src/exec/file-metadata-utils.h File be/src/exec/file-metadata-utils.h: http://gerrit.cloudera.org:8080/#/c/18531/10/be/src/exec/file-metadata-utils.h@42 PS10, Line 42: void Open(RuntimeState* state, const HdfsFileDesc* file_desc); > I am ok with "init" + a comment that it can be called several times :) Actually, SetFile makes more sense after giving it some thought. :) http://gerrit.cloudera.org:8080/#/c/18531/11/be/src/exec/hdfs-scan-node-base.cc File be/src/exec/hdfs-scan-node-base.cc: http://gerrit.cloudera.org:8080/#/c/18531/11/be/src/exec/hdfs-scan-node-base.cc@959 PS11, Line 959: for (const FilterContext& ctx: filter_ctxs) { > In this loop, is it possible that we are evaluating filter that is not actu It should not be possible, the CreateTemplateTuple call in line 954 will populate the template_tuple with the Iceberg transformations. Then in the internal loop line 965 only the filters on the written slots will be evaluated. http://gerrit.cloudera.org:8080/#/c/18531/9/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java File fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java: http://gerrit.cloudera.org:8080/#/c/18531/9/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@116 PS9, Line 116: super.computeNodeResourceProfile(queryOptions); : // Increase t > Hmm, that looks more like an issue to me than something that is actually ne That is a great point, added a new mem_pool and reworked the estimation. http://gerrit.cloudera.org:8080/#/c/18531/11/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java File fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java: http://gerrit.cloudera.org:8080/#/c/18531/11/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@132 PS11, Line 132: numInstances_ > Just an idea: wouldn't it be cleaner if this division was done in calculati Reworked this part with the new mem pool. -- To view, visit http://gerrit.cloudera.org:8080/18531 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7762e1238bdf236b85d2728881a402a2bb41f36a Gerrit-Change-Number: 18531 Gerrit-PatchSet: 11 Gerrit-Owner: Tamas Mate <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Gergely Fürnstáhl <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Tamas Mate <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Wed, 03 Aug 2022 10:36:10 +0000 Gerrit-HasComments: Yes
