Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/18234 )
Change subject: IMPALA-11008: fix incorrect to propagate inferred predicates ...................................................................... Patch Set 7: (13 comments) This is a great fix. http://gerrit.cloudera.org:8080/#/c/18234/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18234/7//COMMIT_MSG@11 PS7, Line 11: condition nit. may remove. http://gerrit.cloudera.org:8080/#/c/18234/7//COMMIT_MSG@27 PS7, Line 27: Because nit. This is incorrect since http://gerrit.cloudera.org:8080/#/c/18234/7//COMMIT_MSG@29 PS7, Line 29: condition nit. remove http://gerrit.cloudera.org:8080/#/c/18234/7/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/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@513 PS7, Line 513: get nit. delete http://gerrit.cloudera.org:8080/#/c/18234/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@514 PS7, Line 514: in nit. be added to http://gerrit.cloudera.org:8080/#/c/18234/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@515 PS7, Line 515: ojNullableEqualSlots nit. maybe ojNullableSlotsInEquiPreds? http://gerrit.cloudera.org:8080/#/c/18234/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2124 PS7, Line 2124: conditions for the nullable side nit. remove http://gerrit.cloudera.org:8080/#/c/18234/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2130 PS7, Line 2130: // We use 'ojmap' to save src to outer join equal slot, eg. B.id. We substitue nit. save the mapping from src to the outer join equal slot. http://gerrit.cloudera.org:8080/#/c/18234/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2150 PS7, Line 2150: if (ojSmap.size() != 0) { : p = p.trySubstitute(ojSmap, this, false); : } I wonder if this is a redundant operation since the content of ojmap is a subset of that of smap. http://gerrit.cloudera.org:8080/#/c/18234/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2729 PS7, Line 2729: (tblRef.getJoinOp() == JoinOperator.LEFT_OUTER_JOIN || : tblRef.getJoinOp() == JoinOperator.RIGHT_OUTER_JOIN Should we handle FULL_OUTER_JOIN and ANTI joins? http://gerrit.cloudera.org:8080/#/c/18234/7/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/7/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test@1593 PS7, Line 1593: # IMPALA-11008: fix incorrect to propagate inferred predicates nit. may add a comment here to state the potential push-down predicate filters on NULL and is thus not push-able. The same argument applies to other new negative tests in this file. http://gerrit.cloudera.org:8080/#/c/18234/7/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test@1603 PS7, Line 1603: LEFT JOIN We may need to add a couple of RIGHT JOIN queries. http://gerrit.cloudera.org:8080/#/c/18234/7/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test@1723 PS7, Line 1723: t2.id + t2.zip = 2 This is cool. -- 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: 7 Gerrit-Owner: Xianqing He <hexianqing...@126.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Xianqing He <hexianqing...@126.com> Gerrit-Comment-Date: Tue, 22 Mar 2022 16:26:05 +0000 Gerrit-HasComments: Yes
