englefly commented on PR #69111:
URL: https://github.com/apache/doris/pull/69111#issuecomment-6209386550
Follow-up commit `8af10b7bee3` (from the discussion about whether keeping a
producer output exprId can leave "same exprId, different nullable" behind).
It cannot, because the boundary outputs are not *re-defined*, they are left
**untouched**: for a CTE producer output the rule neither replaces the grouping
key nor materializes the constant, so the aggregate keeps its own output slot
and the exprId still has exactly one definition with unchanged nullable/type
(only this one column loses the constant materialization; everything else in
the body keeps the fresh exprIds).
While checking that, I found one branch which did not follow the rule: an
output expression that folds to a constant and is *not* a grouping key still
fell back to the folded expression, which is a literal and not a
`NamedExpression` - the column would be dropped from `newOutputExprs` and the
following `checkArgument` would fail. In practice such an expression is planned
in a project above the aggregate, which is guarded too, so I could not turn it
into a real failure (a CTE with `k + 0 AS x ... GROUP BY v, k` passes either
way), but the code now states the intended behaviour explicitly:
```java
if (isCteProducerOutput(expr)) {
// the consumers of the CTE map this output of the producer by its
exprId, so the aggregate has to
// keep the output expression itself: the constant is not materialized
for this column, and since
// the definition does not change, the exprId keeps describing one and
the same column
newOutputExpr = expr;
} else {
newOutputExpr = newGroupByExprSet.contains(expr) ? expr : replacedExpr;
}
```
Added
`ConstantPropagationFreshExprIdTest#testMaterializedCteKeepsConstantProducerOutput`
for that shape (producer keeps its three outputs, consumer maps valid, query
plannable) and factored the consumer-map invariant into a helper used by both
materialized CTE tests. ConstantPropagationTest 11/11,
ConstantPropagationFreshExprIdTest 4/4, checkstyle passes.
--
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]
