Xianqing He has posted comments on this change. ( http://gerrit.cloudera.org:8080/18234 )
Change subject: IMPALA-11008: fix incorrect to propagate inferred predicates ...................................................................... Patch Set 5: (7 comments) Thanks for reviewing the code http://gerrit.cloudera.org:8080/#/c/18234/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18234/4//COMMIT_MSG@15 PS4, Line 15: SELECT * : FROM ( : SELECT id > nit: could you format this a bit for readability? E.g. Done http://gerrit.cloudera.org:8080/#/c/18234/4//COMMIT_MSG@18 PS4, Line 18: : FROM ( > nit: for readability, could you format this a bit? E.g. Done http://gerrit.cloudera.org:8080/#/c/18234/4//COMMIT_MSG@20 PS4, Line 20: A.id, B.col > nit: could you format this a bit as above? Done http://gerrit.cloudera.org:8080/#/c/18234/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: http://gerrit.cloudera.org:8080/#/c/18234/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2009 PS4, Line 2009: } > Is it possible to reject the conjunct here? These codes aim to reject conju The 'srcConjunct' is single tuple id conjunct, if we want to reject here, we need to add the same src-to-dest mapping code as #2036. I think it's appropriate to reject the conjuncts in generating inferred predicates. http://gerrit.cloudera.org:8080/#/c/18234/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2043 PS4, Line 2043: * All other inferred predicates are marked as assigned if 'markAssigned' : * is true. This function returns bound predicates regardless of whether the source : * predicates have been assigned. : * Destination slots in destTid can be ignored by passing them in ignoreSlots. : * Some bound predicates may be missed due to errors in backend expr evaluation : * or expr substitution. : * TODO: exclude UDFs from predicate propagation? their overloaded variants could : * have very different semantics > Could you also add some comments about "ojsmap". I think we want to substit Done http://gerrit.cloudera.org:8080/#/c/18234/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2647 PS4, Line 2647: > Should we only do this for outer joins? Now we also register anti join slot Done http://gerrit.cloudera.org:8080/#/c/18234/4/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test File testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test: http://gerrit.cloudera.org:8080/#/c/18234/4/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test@1617 PS4, Line 1617: | other predicates: CASE WHEN t1.id IS NOT NULL AND t2.some_nulls IS NULL THEN TRUE ELSE NULL END IS NOT NULL > Here we are missing a runtime filter, which causes the test failure: https: Done -- To view, visit http://gerrit.cloudera.org:8080/18234 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e64230f6d0c2b9ef1560186ceba349a5920ccdf Gerrit-Change-Number: 18234 Gerrit-PatchSet: 5 Gerrit-Owner: Xianqing He <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Xianqing He <[email protected]> Gerrit-Comment-Date: Mon, 21 Feb 2022 12:17:42 +0000 Gerrit-HasComments: Yes
