This is an automated email from the ASF dual-hosted git repository. yiguolei pushed a commit to branch branch-2.1 in repository https://gitbox.apache.org/repos/asf/doris.git
commit bfe293c725f2712e8c516e24508f7a643dce6987 Author: starocean999 <[email protected]> AuthorDate: Fri May 24 14:33:12 2024 +0800 [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 | 54 +++++++++++++--------- .../rules/rewrite/PushProjectIntoUnion.java | 5 +- .../nereids/trees/plans/logical/LogicalUnion.java | 7 +++ .../nereids_syntax_p0/adjust_nullable.groovy | 18 ++++++++ 4 files changed, 59 insertions(+), 25 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 a608448e023..0404e7b71c9 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<Map<ExprId, Slot>> imple @Override public Plan visitLogicalSetOperation(LogicalSetOperation setOperation, Map<ExprId, Slot> replaceMap) { setOperation = (LogicalSetOperation) super.visit(setOperation, replaceMap); - if (setOperation.children().isEmpty()) { - return setOperation; - } - List<Boolean> inputNullable = setOperation.child(0).getOutput().stream() - .map(ExpressionTrait::nullable).collect(Collectors.toList()); ImmutableList.Builder<List<SlotReference>> newChildrenOutputs = ImmutableList.builder(); - for (int i = 0; i < setOperation.arity(); i++) { - List<Slot> childOutput = setOperation.child(i).getOutput(); - List<SlotReference> setChildOutput = setOperation.getRegularChildOutput(i); - ImmutableList.Builder<SlotReference> 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<Boolean> 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<Slot> childOutput = setOperation.child(i).getOutput(); + List<SlotReference> setChildOutput = setOperation.getRegularChildOutput(i); + ImmutableList.Builder<SlotReference> 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; + 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<NamedExpression> constantExprs : logicalUnion.getConstantExprsList()) { for (int j = 0; j < constantExprs.size(); j++) { - if (constantExprs.get(j).nullable()) { - inputNullable.set(j, true); - } + 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<NamedExpression> outputs = setOperation.getOutputs(); List<NamedExpression> 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 971071d1a63..130d4b04d3f 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 8c5bbfcc6a6..6f7913369b4 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<NamedExpression> newOutputs, List<Plan> children, + List<List<SlotReference>> childrenOutputs, + List<List<NamedExpression>> 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 24fb20d9aa2..c9f99299220 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") + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
