From e7bc29a0a2784bff06c8c83fe1b12d73b508ddad Mon Sep 17 00:00:00 2001 From: starocean999 <12095047@qq.com> Date: Fri, 24 May 2024 10:48:34 +0800 Subject: [PATCH] [fix](nereids)AdjustNullable rule should handle union node with no children --- .../nereids/rules/rewrite/AdjustNullable.java | 54 +++++++++++-------- .../rules/rewrite/PushProjectIntoUnion.java | 5 +- .../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 a608448e023f07..0404e7b71c9bc3 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 Plan visitLogicalRepeat(LogicalRepeat repeat, 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; + 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++) { - 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 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 971071d1a6391e..130d4b04d3f2fa 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 Rule build() { } 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 8c5bbfcc6a60c5..6f7913369b4a4e 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 LogicalUnion withChildrenAndConstExprsList(List children, 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 24fb20d9aa26c7..c9f99299220ae4 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") + } }