Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/24605 )
Change subject: IMPALA-15195: Calcite Planner performance improvements for tpcds ...................................................................... Patch Set 6: Code-Review+1 (3 comments) Mostly nits at this point. Doing a +1. I will try and do another pass soon. http://gerrit.cloudera.org:8080/#/c/24605/6/fe/src/main/java/org/apache/impala/planner/Planner.java File fe/src/main/java/org/apache/impala/planner/Planner.java: http://gerrit.cloudera.org:8080/#/c/24605/6/fe/src/main/java/org/apache/impala/planner/Planner.java@1022 PS6, Line 1022: avoid all invert joins in its : // optimization phase, nit: This is still a bit confusing .. it is saying 'avoid all invert joins' even though the first 2 if-else blocks are calling joinNode.invertJoin() without checking this new flag. Maybe add a TODO in front of the JIRA reference to indicate this is future plan. http://gerrit.cloudera.org:8080/#/c/24605/6/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/FilterSelectivityEstimator.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/FilterSelectivityEstimator.java: http://gerrit.cloudera.org:8080/#/c/24605/6/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/FilterSelectivityEstimator.java@212 PS6, Line 212: return getNumNulls(columnNum, table) / table.getRowCount(); nit: this assumes that one of the operands of the division is a double. Best to cast the numerator explicitly. http://gerrit.cloudera.org:8080/#/c/24605/6/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/FilterSelectivityEstimator.java@266 PS6, Line 266: The hope nit: this is the exact behavior, so this phrase can be skipped. -- To view, visit http://gerrit.cloudera.org:8080/24605 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ee6662dc6500ea6ebd9f859de4df1740c51a058 Gerrit-Change-Number: 24605 Gerrit-PatchSet: 6 Gerrit-Owner: Steve Carlin <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Tue, 04 Aug 2026 17:46:19 +0000 Gerrit-HasComments: Yes
