Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/16103 )
Change subject: IMPALA-9294: Support DATE for min-max runtime filter ...................................................................... Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/16103/3/testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test File testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test: http://gerrit.cloudera.org:8080/#/c/16103/3/testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test@168 PS3, Line 168: functional_kudu.date_tbl a : join [BROADCAST] functional_kudu.date_tbl b > It's working too. Will add a new test case. While its a little hard to tell and I think that we do have a test that technically hits this path, it would be good to add a functional-query test that is specifically targeted at this, eg. in min_max_filters.test add a "Test case 5" with a query like one of the three-way shuffle joins that I'm asking you to remove from the functional-planner/min-max-runtime-filters.test http://gerrit.cloudera.org:8080/#/c/16103/7/testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test File testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test: http://gerrit.cloudera.org:8080/#/c/16103/7/testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test@167 PS7, Line 167: # Query with date-typed join column I think this case can be left out - its only showing that we'll generate DATE min-max filters now, but we already have tests for that in the functional-query tests and we don't already have planner tests for all of the other min-max filter types. http://gerrit.cloudera.org:8080/#/c/16103/7/testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test@206 PS7, Line 206: # Query with 3-way join on date-typed join columns I think its reasonable to leave this case in as we don't already have a planner test for a three-way join with all Kudu tables, but lets rewrite this comment to say that's what we're testing, i.e. its not the fact that we have DATE columns here that makes this case different from the others (you might even change the type of one of the columns just for variety) its the fact that its a three way join with two different join nodes generating different runtime filters. http://gerrit.cloudera.org:8080/#/c/16103/7/testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test@256 PS7, Line 256: 00:SCAN KUDU [functional_kudu.date_tbl a] I don't think this case is testing anything different from the existing case above labeled "Query with both Kudu and HDFS targets" and so it can be removed. http://gerrit.cloudera.org:8080/#/c/16103/7/testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test@312 PS7, Line 312: runtime filters: RF001[min_max] -> a.id_col, RF003[min_max] -> a.date_col Same as above, I don't think this case is testing anything different from the existing case above labeled "Query with both Kudu and HDFS targets" and so it can be removed. -- To view, visit http://gerrit.cloudera.org:8080/16103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic2f6e2dc6949735d5f0fcf317361cc2969a5e82c Gerrit-Change-Number: 16103 Gerrit-PatchSet: 6 Gerrit-Owner: Wenzhe Zhou <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Comment-Date: Tue, 07 Jul 2020 18:07:41 +0000 Gerrit-HasComments: Yes
