Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20993 )
Change subject: IMPALA-12784: Fix bug in ScanNode.getFilteredInputCardinality ...................................................................... Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/20993/1/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/20993/1/fe/src/main/java/org/apache/impala/planner/ScanNode.java@338 PS1, Line 338: return getLimit(); > not related to the current fix, but shouldn't we return min(limit, inputCar I think that make sense. However, I'd rather investigate it in separate patch since, depending on EXEC_SINGLE_NODE_ROWS_THRESHOLD option, it has risk triggering small query optimization (run on coordinator only instead of distributing it). I'd like to also keep it match with getInputCardinality(). The limit consideration is first implemented in IMPALA-165 and then corrected by IMPALA-5602. The only possibility where a limit is set for ScanNode is when there is only 1 fragment planned by SingleNodePlanner, set here: https://github.com/apache/impala/blob/f87c20800de9f7dc74e47aa9a8c0dc878f4f0840/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java#L326 Setting a limit is also usually driven by user own knowledge that the table has many more rows than the specified limit. -- To view, visit http://gerrit.cloudera.org:8080/20993 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iae41f0fe5907cfed6ac67c156e975cf9964a7559 Gerrit-Change-Number: 20993 Gerrit-PatchSet: 2 Gerrit-Owner: Riza Suminto <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Mon, 05 Feb 2024 18:19:26 +0000 Gerrit-HasComments: Yes
