From bfe293c725f2712e8c516e24508f7a643dce6987 Mon Sep 17 00:00:00 2001 From: starocean999 <40539150+starocean999@users.noreply.github.com> Date: Fri, 24 May 2024 14:33:12 +0800 Subject: [PATCH] [fix](nereids) AdjustNullable rule should handle union node with no children (#35074) The output slot's nullable info is not correctly calculated in union node. Because old code only get correct result if union node has children. But the union node may have no children but only have constantExprList. So in that case, we should calculate output's nullable info byboth children and constantExprList. --- .../nereids/rules/rewrite/AdjustNullable.java | 58 +++++++++++-------- .../rules/rewrite/PushProjectIntoUnion.java | 5 +- .../trees/plans/logical/LogicalUnion.java | 7 +++ .../nereids_syntax_p0/adjust_nullable.groovy | 18 ++++++ 4 files changed, 61 insertions(+), 27 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AdjustNullable.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AdjustNullable.java index a608448e02..0404e7b71c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AdjustNullable.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AdjustNullable.java @@ -165,45 +165,55 @@ public class AdjustNullable extends DefaultPlanRewriter> imple @Override public Plan visitLogicalSetOperation(LogicalSetOperation setOperation, Map replaceMap) { setOperation = (LogicalSetOperation) super.visit(setOperation, replaceMap); - if (setOperation.children().isEmpty()) { - return setOperation; - } - List inputNullable = setOperation.child(0).getOutput().stream() - .map(ExpressionTrait::nullable).collect(Collectors.toList()); ImmutableList.Builder> newChildrenOutputs = ImmutableList.builder(); - for (int i = 0; i < setOperation.arity(); i++) { - List childOutput = setOperation.child(i).getOutput(); - List setChildOutput = setOperation.getRegularChildOutput(i); - ImmutableList.Builder newChildOutputs = ImmutableList.builder(); - for (int j = 0; j < setChildOutput.size(); j++) { - for (Slot slot : childOutput) { - if (slot.getExprId().equals(setChildOutput.get(j).getExprId())) { - inputNullable.set(j, slot.nullable() || inputNullable.get(j)); - newChildOutputs.add((SlotReference) slot); - break; + List inputNullable = null; + if (!setOperation.children().isEmpty()) { + inputNullable = setOperation.child(0).getOutput().stream() + .map(ExpressionTrait::nullable).collect(Collectors.toList()); + for (int i = 0; i < setOperation.arity(); i++) { + List childOutput = setOperation.child(i).getOutput(); + List setChildOutput = setOperation.getRegularChildOutput(i); + ImmutableList.Builder newChildOutputs = ImmutableList.builder(); + for (int j = 0; j < setChildOutput.size(); j++) { + for (Slot slot : childOutput) { + if (slot.getExprId().equals(setChildOutput.get(j).getExprId())) { + inputNullable.set(j, slot.nullable() || inputNullable.get(j)); + newChildOutputs.add((SlotReference) slot); + break; + } } } + newChildrenOutputs.add(newChildOutputs.build()); } - newChildrenOutputs.add(newChildOutputs.build()); } if (setOperation instanceof LogicalUnion) { LogicalUnion logicalUnion = (LogicalUnion) setOperation; - for (List constantExprs : logicalUnion.getConstantExprsList()) { - for (int j = 0; j < constantExprs.size(); j++) { - if (constantExprs.get(j).nullable()) { - inputNullable.set(j, true); - } + if (!logicalUnion.getConstantExprsList().isEmpty() && setOperation.children().isEmpty()) { + int outputSize = logicalUnion.getConstantExprsList().get(0).size(); + // create the inputNullable list and fill it with all FALSE values + inputNullable = Lists.newArrayListWithCapacity(outputSize); + for (int i = 0; i < outputSize; i++) { + inputNullable.add(false); } } + for (List constantExprs : logicalUnion.getConstantExprsList()) { + for (int j = 0; j < constantExprs.size(); j++) { + inputNullable.set(j, inputNullable.get(j) || constantExprs.get(j).nullable()); + } + } + } + if (inputNullable == null) { + // this is a fail-safe + // means there is no children and having no getConstantExprsList + // no way to update the nullable flag, so just do nothing + return setOperation; } List outputs = setOperation.getOutputs(); List newOutputs = Lists.newArrayListWithCapacity(outputs.size()); for (int i = 0; i < inputNullable.size(); i++) { NamedExpression ne = outputs.get(i); Slot slot = ne instanceof Alias ? (Slot) ((Alias) ne).child() : (Slot) ne; - if (inputNullable.get(i)) { - slot = slot.withNullable(true); - } + slot = slot.withNullable(inputNullable.get(i)); newOutputs.add(ne instanceof Alias ? (NamedExpression) ne.withChildren(slot) : slot); } newOutputs.forEach(o -> replaceMap.put(o.getExprId(), o.toSlot())); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushProjectIntoUnion.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushProjectIntoUnion.java index 971071d1a6..130d4b04d3 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushProjectIntoUnion.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushProjectIntoUnion.java @@ -67,9 +67,8 @@ public class PushProjectIntoUnion extends OneRewriteRuleFactory { } newConstExprs.add(newProjections.build()); } - return p.child() - .withChildrenAndConstExprsList(ImmutableList.of(), ImmutableList.of(), newConstExprs.build()) - .withNewOutputs(p.getOutputs()); + return p.child().withNewOutputsChildrenAndConstExprsList(ImmutableList.copyOf(p.getOutput()), + ImmutableList.of(), ImmutableList.of(), newConstExprs.build()); }).toRule(RuleType.PUSH_PROJECT_INTO_UNION); } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalUnion.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalUnion.java index 8c5bbfcc6a..6f7913369b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalUnion.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalUnion.java @@ -180,6 +180,13 @@ public class LogicalUnion extends LogicalSetOperation implements Union, OutputPr return new LogicalUnion(qualifier, outputs, childrenOutputs, constantExprsList, hasPushedFilter, children); } + public LogicalUnion withNewOutputsChildrenAndConstExprsList(List newOutputs, List children, + List> childrenOutputs, + List> constantExprsList) { + return new LogicalUnion(qualifier, newOutputs, childrenOutputs, constantExprsList, + hasPushedFilter, Optional.empty(), Optional.empty(), children); + } + public LogicalUnion withAllQualifier() { return new LogicalUnion(Qualifier.ALL, outputs, regularChildrenOutputs, constantExprsList, hasPushedFilter, Optional.empty(), Optional.empty(), children); diff --git a/regression-test/suites/nereids_syntax_p0/adjust_nullable.groovy b/regression-test/suites/nereids_syntax_p0/adjust_nullable.groovy index 24fb20d9aa..c9f9929922 100644 --- a/regression-test/suites/nereids_syntax_p0/adjust_nullable.groovy +++ b/regression-test/suites/nereids_syntax_p0/adjust_nullable.groovy @@ -95,5 +95,23 @@ suite("adjust_nullable") { sql """ drop table if exists table_8_undef_undef; """ + + sql """ + drop table if exists orders_2_x; + """ + + sql """CREATE TABLE `orders_2_x` ( + `o_orderdate` DATE not NULL + ) ENGINE=OLAP + DUPLICATE KEY(`o_orderdate`) + DISTRIBUTED BY HASH(`o_orderdate`) BUCKETS 96 + PROPERTIES ( + "replication_allocation" = "tag.location.default: 1" + );""" + + explain { + sql("verbose insert into orders_2_x values ( '2023-10-17'),( '2023-10-17');") + notContains("nullable=true") + } }