github-actions[bot] commented on code in PR #69074:
URL: https://github.com/apache/doris/pull/69074#discussion_r4271005708
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AdjustNullable.java:
##########
@@ -289,25 +291,61 @@ public Plan visitLogicalSetOperation(LogicalSetOperation
setOperation, Map<ExprI
ImmutableList.Builder<List<SlotReference>> newChildrenOutputs =
ImmutableList.builder();
List<Boolean> inputNullable = null;
if (!setOperation.children().isEmpty()) {
- inputNullable =
Lists.newArrayListWithCapacity(setOperation.getOutputs().size());
- for (int i = 0; i < setOperation.getOutputs().size(); i++) {
- inputNullable.add(false);
+ // whether the output column can be null depends on the semantics
of the set operation:
+ // - union: a row is in the result if it appears in any child, so
the column can be null if any
+ // child column can be null;
+ // - intersect: a row is in the result only if it appears in every
child, so the column can be
+ // null only if every child column can be null;
+ // - except: the result is a subset of the left child(child(0)),
so the column can be null only
+ // if the left child column can be null.
+ boolean isIntersect = setOperation instanceof LogicalIntersect;
+ boolean isExcept = setOperation instanceof LogicalExcept;
+ int outputSize = setOperation.getOutputs().size();
+ inputNullable = Lists.newArrayListWithCapacity(outputSize);
+ List<Boolean> foundAllChildren =
Lists.newArrayListWithCapacity(outputSize);
+ for (int i = 0; i < outputSize; i++) {
+ // true is the identity value of the "and" used by intersect,
+ // false is the identity value of the "or" used by union
+ inputNullable.add(isIntersect);
+ foundAllChildren.add(true);
}
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++) {
+ Slot childSlot = null;
for (Slot slot : childOutput) {
if
(slot.getExprId().equals(setChildOutput.get(j).getExprId())) {
- inputNullable.set(j, slot.nullable() ||
inputNullable.get(j));
- newChildOutputs.add((SlotReference) slot);
+ childSlot = slot;
break;
}
}
+ if (childSlot == null) {
+ foundAllChildren.set(j, false);
+ continue;
+ }
+ newChildOutputs.add((SlotReference) childSlot);
+ if (isIntersect) {
+ inputNullable.set(j, inputNullable.get(j) &&
childSlot.nullable());
Review Comment:
[P1] Update BE output handling before narrowing set-operation nullability
For populated tables `t_nullable(k INT NULL)` and `t_not_null(k INT NOT
NULL)`, this produces the following reduced plan for `SELECT k FROM t_nullable
INTERSECT SELECT k FROM t_not_null`:
```text
Intersect(output s NOT NULL)
Scan(t_nullable.k NULL)
Scan(t_not_null.k NOT NULL)
```
`visitLogicalSetOperation` keeps the original child slots but makes `s`
non-null. The implementation rules and
`PhysicalPlanTranslator.visitPhysicalSetOperation` copy that non-null result
into the output tuple and keep the nullable child expressions. However,
`SetSharedState::update_build_not_ignore_null`
(`be/src/exec/pipeline/dependency.cpp:503-512`) still ORs nullability from all
inputs, yielding true, and `SetSourceLocalState::open`
(`be/src/exec/operator/set_source_operator.cpp:50-65`) unconditionally rejects
the false output flag versus that true flag. Thus a valid query that previously
returned matching rows now fails with `SET operator expects a nullalbe : false
... output is a nullable : true`, independently of `fe_debug` or whether the
data actually contains NULL. `SELECT k FROM t_not_null EXCEPT SELECT k FROM
t_nullable` fails for the same reason.
Please coordinate this narrowing with BE hash-key/output-column handling and
add populated execution regressions for both operations. Removing only the
equality check is insufficient: the build columns are still stored/wrapped as
nullable and copied by the source without a non-null output conversion. The new
tests stop at `analyze().rewrite()` and cannot catch this execution failure.
--
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]
