github-actions[bot] commented on code in PR #66806:
URL: https://github.com/apache/doris/pull/66806#discussion_r3842107661
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/EliminateJoinCondition.java:
##########
@@ -19,36 +19,89 @@
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.expressions.Alias;
import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.expressions.StatementScopeIdGenerator;
import org.apache.doris.nereids.trees.expressions.literal.BooleanLiteral;
+import org.apache.doris.nereids.trees.expressions.literal.NullLiteral;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.trees.plans.logical.LogicalEmptyRelation;
+import org.apache.doris.nereids.trees.plans.logical.LogicalJoin;
+import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
+
+import com.google.common.collect.ImmutableList;
import java.util.List;
+import java.util.Set;
import java.util.stream.Collectors;
/**
- * Eliminate true Condition in Join Condition.
+ * Eliminate constant conditions in Join Condition.
*/
public class EliminateJoinCondition extends OneRewriteRuleFactory {
@Override
public Rule build() {
- return logicalJoin().then(join -> {
- List<Expression> hashJoinConjuncts =
join.getHashJoinConjuncts().stream()
- .filter(expression ->
!expression.equals(BooleanLiteral.TRUE))
- .collect(Collectors.toList());
- List<Expression> otherJoinConjuncts =
join.getOtherJoinConjuncts().stream()
- .filter(expression ->
!expression.equals(BooleanLiteral.TRUE))
- .collect(Collectors.toList());
- List<Expression> markJoinConjuncts =
join.getMarkJoinConjuncts().stream()
- .filter(expression ->
!expression.equals(BooleanLiteral.TRUE))
- .collect(Collectors.toList());
- if (hashJoinConjuncts.size() == join.getHashJoinConjuncts().size()
- && otherJoinConjuncts.size() ==
join.getOtherJoinConjuncts().size()
- && markJoinConjuncts.size() ==
join.getMarkJoinConjuncts().size()) {
- return null;
+ return logicalJoin()
+ .then(EliminateJoinCondition::eliminateJoinCondition)
+ .toRule(RuleType.ELIMINATE_JOIN_CONDITION);
+ }
+
+ static Plan eliminateJoinCondition(LogicalJoin<? extends Plan, ? extends
Plan> join) {
+ List<Expression> hashJoinConjuncts =
removeTrueConjuncts(join.getHashJoinConjuncts());
+ List<Expression> otherJoinConjuncts =
removeTrueConjuncts(join.getOtherJoinConjuncts());
+ List<Expression> markJoinConjuncts =
removeTrueConjuncts(join.getMarkJoinConjuncts());
+
+ if (!join.isMarkJoin() && (containsFalseOrNull(hashJoinConjuncts)
+ || containsFalseOrNull(otherJoinConjuncts))) {
+ switch (join.getJoinType()) {
+ case INNER_JOIN:
+ case CROSS_JOIN:
+ return new
LogicalEmptyRelation(StatementScopeIdGenerator.newRelationId(),
join.getOutput());
+ case LEFT_OUTER_JOIN:
+ return projectNullPaddedJoinOutput(join, join.left());
+ case RIGHT_OUTER_JOIN:
+ return projectNullPaddedJoinOutput(join, join.right());
+ default:
+ break;
+ }
+ }
+
+ if (hashJoinConjuncts.size() == join.getHashJoinConjuncts().size()
+ && otherJoinConjuncts.size() ==
join.getOtherJoinConjuncts().size()
+ && markJoinConjuncts.size() ==
join.getMarkJoinConjuncts().size()) {
+ return null;
+ }
+ return join.withJoinConjuncts(hashJoinConjuncts, otherJoinConjuncts,
markJoinConjuncts,
+ join.getJoinReorderContext());
+ }
+
+ private static List<Expression> removeTrueConjuncts(List<Expression>
conjuncts) {
+ return conjuncts.stream()
+ .filter(expression -> !expression.equals(BooleanLiteral.TRUE))
+ .collect(Collectors.toList());
+ }
+
+ private static boolean containsFalseOrNull(List<Expression> conjuncts) {
+ return conjuncts.stream()
+ .anyMatch(expression ->
expression.equals(BooleanLiteral.FALSE) || expression.isNullLiteral());
+ }
+
+ private static LogicalProject<Plan> projectNullPaddedJoinOutput(
+ LogicalJoin<? extends Plan, ? extends Plan> join, Plan
preservedChild) {
+ Set<Slot> preservedOutput = preservedChild.getOutputSet();
+ ImmutableList.Builder<NamedExpression> projects =
+ ImmutableList.builderWithExpectedSize(join.getOutput().size());
+ for (Slot output : join.getOutput()) {
+ if (preservedOutput.contains(output)) {
+ projects.add(output);
+ } else {
+ projects.add(new Alias(output.getExprId(),
ImmutableList.of(new NullLiteral(output.getDataType())),
+ output.getName(), output.getQualifier(), false));
Review Comment:
[P1] Keep typed NULL traits from erasing ordinary equality
With `disable_nereids_rules=INFER_JOIN_NOT_NULL`, two `LEFT JOIN ... ON
FALSE` children both reach this projection and publish present uniform
`NullLiteral`s. A parent inner join on the two padded slots survives constant
propagation, then late `EliminateConstHashJoinCondition` sees the two literals
as Java-equal and removes the only `EqualTo`; the nested-loop join is left
unconditioned and returns Cartesian rows even though SQL `NULL = NULL` is
UNKNOWN. Please make that cleanup require equal non-NULL uniforms (or otherwise
eliminate this parent as empty), and add an end-to-end regression with this
rule disabled.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
