Aleksandr Efimov has posted comments on this change. ( http://gerrit.cloudera.org:8080/24597 )
Change subject: IMPALA-7996: Optimize constant-false outer joins ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/24597/3/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java File fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java: http://gerrit.cloudera.org:8080/#/c/24597/3/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java@459 PS3, Line 459: boolean useRandomPartitioning = node.isConstantFalseOuterJoin(); > So it's good to prevent data skew and I wouldn't hold up the review for thi Agreed. Fixed in PS4. For a constant-false LEFT OUTER JOIN, the non-preserved build input is empty, so we now ignore SHUFFLE and broadcast it. For RIGHT OUTER JOIN, we first invert the join so the empty original left input becomes the broadcast build. FULL OUTER JOIN stays partitioned because both inputs are preserved; broadcasting the build would duplicate its unmatched rows across join instances. Its inputs remain RANDOM-partitioned to avoid constant-key skew. http://gerrit.cloudera.org:8080/#/c/24597/3/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java: http://gerrit.cloudera.org:8080/#/c/24597/3/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@2118 PS3, Line 2118: e -> Expr.IS_FALSE_LITERAL.apply(e) || Expr.IS_NULL_VALUE.apply(e)); > I guess this is ok and safer to go through all the conjuncts , but I do won Expression rewriting folds the individual constant conjunct, but the analyzer currently keeps outer-join ON conjuncts as separately registered predicates: a = b remains in equiJoinConjuncts, while FALSE or typed NULL remains in otherJoinConjuncts. It does not replace that registered set with one FALSE predicate, so scanning the remaining ON conjuncts is necessary with the current representation. PS4 covers literal and folded FALSE as well as typed NULL. http://gerrit.cloudera.org:8080/#/c/24597/3/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@2121 PS3, Line 2121: eqJoinConjuncts.add(createNonMatchingEqJoinConjunct(analyzer)); > So the join conjunct which prolly gets classified in "other" will contain t The original FALSE predicate is not moved into equiJoinConjuncts. We replace the usable keys with a synthetic TRUE = FALSE key. Without an equi conjunct, ON FALSE is planned as a NestedLoopJoinNode; the non-matching hash key keeps it as a HashJoinNode so the empty-input/broadcast and random-partitioning optimizations can be used. Since that key can never match, PS4 also removes the original other predicate. http://gerrit.cloudera.org:8080/#/c/24597/3/testdata/workloads/functional-planner/queries/PlannerTest/empty.test File testdata/workloads/functional-planner/queries/PlannerTest/empty.test: http://gerrit.cloudera.org:8080/#/c/24597/3/testdata/workloads/functional-planner/queries/PlannerTest/empty.test@162 PS3, Line 162: | other join predicates: FALSE > I referred to this in my other comment, but if we wouldn't need two predica Agreed. Fixed in PS4: after marking the join as non-matching, we clear otherJoinConjuncts. EXPLAIN now shows only the synthetic hash predicate, and the original FALSE or NULL predicate is not evaluated at runtime. -- To view, visit http://gerrit.cloudera.org:8080/24597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6505825fc958720eec12e13d546661834f221b80 Gerrit-Change-Number: 24597 Gerrit-PatchSet: 3 Gerrit-Owner: Aleksandr Efimov <[email protected]> Gerrit-Reviewer: Aleksandr Efimov <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Paul Rogers <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Comment-Date: Wed, 29 Jul 2026 19:01:39 +0000 Gerrit-HasComments: Yes
