Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/16346 )
Change subject: IMPALA-10064: Support constant propagation for eligible range predicates ...................................................................... Patch Set 8: (4 comments) Looks good to me! http://gerrit.cloudera.org:8080/#/c/16346/8/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/16346/8/fe/src/main/java/org/apache/impala/analysis/ConstantPredicateHandler.java@78 PS8, Line 78: isRangeOp && !(constant instanceof DateLiteral || : constant instanceof TimestampLiteral)) For extensibility, maybe we could add a new argument to the method to indicate the type(s) of predicate to handle, rather hard-code the date and timestamp here. The names of the data members in this class probably will be renamed as well. http://gerrit.cloudera.org:8080/#/c/16346/8/fe/src/main/java/org/apache/impala/analysis/ConstantPredicateHandler.java@93 PS8, Line 93: Propagate equality constant predicates to other conjuncts. Propagate : * range constant predicates to conjuncts involving date and timestamp : * columns. I wonder if the propagation here happens only for the predicates appear in one place (say in the where clause in your example). propagation is also possible cross inner joins. select count(*) from t, s where t.a = s.b and t.a = 2; http://gerrit.cloudera.org:8080/#/c/16346/8/fe/src/main/java/org/apache/impala/analysis/Expr.java File fe/src/main/java/org/apache/impala/analysis/Expr.java: http://gerrit.cloudera.org:8080/#/c/16346/8/fe/src/main/java/org/apache/impala/analysis/Expr.java@1200 PS8, Line 1200: conjuncts, candidates May be add an example here. Specifying what are contained within these data structures are useful. http://gerrit.cloudera.org:8080/#/c/16346/8/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/16346/8/testdata/workloads/functional-planner/queries/PlannerTest/constant-propagation.test@410 PS8, Line 410: select * from functional_parquet.alltypes_date_partition : where date_col = cast(timestamp_col as date) : and timestamp_col between '2009-12-01' and '2010-12-01'; Can we add one more test here? select * from functional_parquet.alltypes_date_partition where date_col = cast(timestamp_col as date) and timestamp_col <= '2009-12-01' or timestamp_col > '2010-12-01'; At least on paper, the propagation should take place :-). -- To view, visit http://gerrit.cloudera.org:8080/16346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I811a1f8d605c27c7704d7fc759a91510c6db3c2b Gerrit-Change-Number: 16346 Gerrit-PatchSet: 8 Gerrit-Owner: Aman Sinha <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Reviewer: Shant Hovsepian <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Fri, 28 Aug 2020 14:27:31 +0000 Gerrit-HasComments: Yes
