Wenzhe Zhou 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 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/16103/3/be/src/runtime/date-value.h File be/src/runtime/date-value.h: http://gerrit.cloudera.org:8080/#/c/16103/3/be/src/runtime/date-value.h@179 PS3, Line 179: pvalue > Maybe do a DCHECK()? The code is copied from timestamp-value.h. The ToColumnValuePB() in the similar classes, like TimestampValue and DecimalValue don't have DCHECK(). http://gerrit.cloudera.org:8080/#/c/16103/3/be/src/util/min-max-filter-test.cc File be/src/util/min-max-filter-test.cc: http://gerrit.cloudera.org:8080/#/c/16103/3/be/src/util/min-max-filter-test.cc@371 PS3, Line 371: DATE_TIME_CHECK_VALS > From the content of this macro, I figured that logic verifies that the filt In this test source file, there are different CheckXxxVals for different type of columns, like CheckIntVals, CheckStringVals, etc. We should keep naming consistent with existing code. http://gerrit.cloudera.org:8080/#/c/16103/3/be/src/util/min-max-filter.h File be/src/util/min-max-filter.h: http://gerrit.cloudera.org:8080/#/c/16103/3/be/src/util/min-max-filter.h@220 PS3, Line 220: > OK. I think the default constructor on min/max is called. Right. When always_false is true, min/max values are invalid. 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 > My concern is whether the above mentioned 3-way join queries work, based on Ok, I will add new 3-way join test case if I need to submit new patch. -- 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: 5 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: Mon, 29 Jun 2020 22:50:01 +0000 Gerrit-HasComments: Yes
