Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-1286: Extract common conjuncts from disjunctions. ......................................................................
Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/4877/1/fe/src/main/java/org/apache/impala/rewrite/ExtractCommonConjunctRule.java File fe/src/main/java/org/apache/impala/rewrite/ExtractCommonConjunctRule.java: Line 42: if (!Expr.IS_OR_PREDICATE.apply(expr)) return expr; > Having separate rules for normalization certainly makes sense, but I see th Ah makes sense. I misunderstood the getConjuncts() call. Thanks for clearing it up. http://gerrit.cloudera.org:8080/#/c/4877/1/testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test File testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test: Line 3384: p_brand = 'Brand#12' > Agree we need normalization as a separate set of rules. Thanks. I think I understand the scope of this fix now. My example was just to show a complex predicate and I didn't check if normalization would apply specifically to that case. Also, if you think its fine, we should probably document a TODO somewhere in ExtractCommonConjunctRule the cases it can't deal without normalization, till we actually implement normalization as a separate step. -- To view, visit http://gerrit.cloudera.org:8080/4877 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3cf9b950afaa3fd753d1b09ba5e540b5258940ad Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
