Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21687 )
Change subject: IMPALA-5443: Apply codegen rows threshold per node ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/21687/3/fe/src/main/java/org/apache/impala/planner/PlanNode.java File fe/src/main/java/org/apache/impala/planner/PlanNode.java: http://gerrit.cloudera.org:8080/#/c/21687/3/fe/src/main/java/org/apache/impala/planner/PlanNode.java@1359 PS3, Line 1359: long in = getInputCardinality(); : long out = getCardinality(); Both of these can be pretty much speculative, for example if the input node has some predicate. My concern is that underestimating cardinality can lead to regression by turning off codegen when it would be actually useful. This would be more work, but I think that ideally an "hardCardinalityLimit" or something similar would be propagated between nodes, similarly to hasHardEstimates_. http://gerrit.cloudera.org:8080/#/c/21687/3/fe/src/main/java/org/apache/impala/planner/ScanNode.java File fe/src/main/java/org/apache/impala/planner/ScanNode.java: http://gerrit.cloudera.org:8080/#/c/21687/3/fe/src/main/java/org/apache/impala/planner/ScanNode.java@656 PS3, Line 656: isAccessingCollectionType() || (missingStats && !hasSimpleLimit() This looks similar to hasHardEstimates_. It doesn't check isTableMissingStats() / hasCorruptTableStats(), but it probable should. http://gerrit.cloudera.org:8080/#/c/21687/3/fe/src/main/java/org/apache/impala/util/PlanNodeCodegenVisitor.java File fe/src/main/java/org/apache/impala/util/PlanNodeCodegenVisitor.java: http://gerrit.cloudera.org:8080/#/c/21687/3/fe/src/main/java/org/apache/impala/util/PlanNodeCodegenVisitor.java@57 PS3, Line 57: numRowsPerInstance numRowsPerNode seems clearer -- To view, visit http://gerrit.cloudera.org:8080/21687 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1b34d4f2ef0d98fcd918a8a546acde70e09d18d7 Gerrit-Change-Number: 21687 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Comment-Date: Wed, 21 Aug 2024 13:59:24 +0000 Gerrit-HasComments: Yes
