Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19572 )
Change subject: IMPALA-11960: Fix constant propagation from TIMESTAMP to DATE ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/19572/2/fe/src/main/java/org/apache/impala/analysis/ConstantPredicateHandler.java File fe/src/main/java/org/apache/impala/analysis/ConstantPredicateHandler.java: http://gerrit.cloudera.org:8080/#/c/19572/2/fe/src/main/java/org/apache/impala/analysis/ConstantPredicateHandler.java@179 PS2, Line 179: if (newOp == BinaryPredicate.Operator.LT) { > Will this change of operand be performed for every type? Shouldn't it only This is only called with date and timestamp arguments, see line 125-130. I agree that the code could be clearer, but I would prefer to do too many changes as this fix will have to be backported to several branches. http://gerrit.cloudera.org:8080/#/c/19572/2/testdata/workloads/functional-planner/queries/PlannerTest/constant-propagation.test File testdata/workloads/functional-planner/queries/PlannerTest/constant-propagation.test: http://gerrit.cloudera.org:8080/#/c/19572/2/testdata/workloads/functional-planner/queries/PlannerTest/constant-propagation.test@439 PS2, Line 439: with dp_view as > This test is good. Could you also add one variant of this which has a mix Done http://gerrit.cloudera.org:8080/#/c/19572/2/testdata/workloads/functional-query/queries/QueryTest/range-constant-propagation.test File testdata/workloads/functional-query/queries/QueryTest/range-constant-propagation.test: http://gerrit.cloudera.org:8080/#/c/19572/2/testdata/workloads/functional-query/queries/QueryTest/range-constant-propagation.test@14 PS2, Line 14: select count(*), sum(int_col) from alltypes_date_partition > Same suggestion as above for the end-to-end test. done with a modified lower bound to match with one of the rows -- To view, visit http://gerrit.cloudera.org:8080/19572 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1938bf5e91057b220daf8a1892940f674aac3d68 Gerrit-Change-Number: 19572 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Comment-Date: Fri, 03 Mar 2023 15:27:47 +0000 Gerrit-HasComments: Yes
