Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20498 )
Change subject: IMPALA-12018: Consider runtime filter for cardinality reduction ...................................................................... Patch Set 12: (5 comments) http://gerrit.cloudera.org:8080/#/c/20498/12/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/20498/12/fe/src/main/java/org/apache/impala/planner/ScanNode.java@514 PS12, Line 514: long buildKeyNdv = filter.getNdvEstimate(); I think that it would be more readable if the the body of the loop would be a separate functions - if I understand correctly it has a pretty clear role of counting a cardinality per filter and updating the partitionSelectivities. It may also make sense to move the function to RuntimeFilter, e.g. CalculateSelectivityForScanNode(). http://gerrit.cloudera.org:8080/#/c/20498/12/fe/src/main/java/org/apache/impala/planner/ScanNode.java@531 PS12, Line 531: If column name is unknown, compare the selectivity : // against filters from other unknown column (colName == ""). I am not sure if this is correct - all max selectivities per column will be multiplied later, and it is possible that the "unknown" selectivity is related to a column already in the list, just wrapped in some expression. It may be safer to ignore these partition filters for now. http://gerrit.cloudera.org:8080/#/c/20498/12/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-cardinality-reduction.test File testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-cardinality-reduction.test: http://gerrit.cloudera.org:8080/#/c/20498/12/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-cardinality-reduction.test@11 PS12, Line 11: ss_sold_date_sk = d_date_sk Can you also add a test when there is an expression in the key to test relevant path in the code? Also, a similar test could be added with a partition key. http://gerrit.cloudera.org:8080/#/c/20498/12/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-cardinality-reduction.test@91 PS12, Line 91: partitions=1824/182 Is it possible to see the effect of runtime filters on partition selectivity in some test? http://gerrit.cloudera.org:8080/#/c/20498/12/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-cardinality-reduction.test@299 PS12, Line 299: left Can you also add a test for semi left join? Also, it seem strange that left anti join is supported as it is not included in isSelectiveAndReducingJoin. -- To view, visit http://gerrit.cloudera.org:8080/20498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I033789c9b63a8188484e3afde8e646563918b3e1 Gerrit-Change-Number: 20498 Gerrit-PatchSet: 12 Gerrit-Owner: Riza Suminto <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: David Rorke <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Mon, 20 Nov 2023 18:13:16 +0000 Gerrit-HasComments: Yes
