Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/22815 )
Change subject: IMPALA-13991: Skip CROSS_JOIN rewrite if subquery in disjuctive expr ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/22815/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22815/3//COMMIT_MSG@11 PS3, Line 11: disjuctive typo http://gerrit.cloudera.org:8080/#/c/22815/3/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java File fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java: http://gerrit.cloudera.org:8080/#/c/22815/3/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java@674 PS3, Line 674: joinConjunct about the joinConjunct = null case: I tried to understand createJoinConjunct and it seems that it only returns null for (NOT) EXISTS predicate. This makes sense to me, as in all other cases the result of the subquery is actually used in the predicate, while EXISTS is turned into a semi join without on predicate. EXISTS within a disjunct is not allowed: IMPALA-9931 So I think that the code could be simplified to use !isDisjunctive instead of !isDisjunctiveAndHasOnClause and add a precondition here for joinConjunct != null If IMPALA-9931 will be implemented then I think that it will actually need a joinConjunct to be able to evaluate the different branches of the disjunct. http://gerrit.cloudera.org:8080/#/c/22815/3/testdata/workloads/functional-planner/queries/PlannerTest/nested-loop-join.test File testdata/workloads/functional-planner/queries/PlannerTest/nested-loop-join.test: http://gerrit.cloudera.org:8080/#/c/22815/3/testdata/workloads/functional-planner/queries/PlannerTest/nested-loop-join.test@364 PS3, Line 364: 89B probably not related to the patch, but I don't get why row-size is 89B - this should only read 2 columns and row-size is 20B in my local env + same for row-size=105B in 03:NESTED LOOP JOIN http://gerrit.cloudera.org:8080/#/c/22815/3/testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test File testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test: http://gerrit.cloudera.org:8080/#/c/22815/3/testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test@228 PS3, Line 228: disjuctive typo -- To view, visit http://gerrit.cloudera.org:8080/22815 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iac0deb0b2fb1536684cce2e004156a20b769b9ab Gerrit-Change-Number: 22815 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Fri, 25 Apr 2025 07:12:28 +0000 Gerrit-HasComments: Yes
