github-actions[bot] commented on code in PR #67152:
URL: https://github.com/apache/doris/pull/67152#discussion_r3864355004
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/FillUpQualifyMissingSlot.java:
##########
@@ -115,10 +121,11 @@ public List<Rule> buildRules() {
qualify -> agg
*/
RuleType.FILL_UP_QUALIFY_AGGREGATE.build(
- logicalQualify(aggregate()).then(qualify -> {
+ logicalQualify(aggregate()).thenApply(ctx -> {
+ LogicalQualify<Aggregate<Plan>> qualify = ctx.root;
checkWindow(qualify);
Aggregate<Plan> agg = qualify.child();
- Resolver resolver = new Resolver(agg);
+ Resolver resolver = new Resolver(agg,
ctx.cascadesContext.getOuterScope());
qualify.getConjuncts().forEach(expr ->
resolver.resolve(expr, ResolvePlanType.QUALIFY));
Review Comment:
[P1] Resolve grouped output aliases to their outer producer
Passing `outerScope` to `Resolver` does not expose an outer dependency
hidden by an aggregate-output alias. A reduced legal EXISTS subquery is:
```text
Qualify[f = 1, rn = 1]
Aggregate[groupBy=i.k; output=i.k, o.flag AS f, window AS rn]
Scan(i)
```
`Resolver.lookUp(f)` finds the local output slot and records only `f -> f`;
it adds no aggregate output and this rule returns unchanged.
`NormalizeAggregate` later encounters `o.flag` in the output project without a
child/group producer, so analysis/final slot validation fails. The added tests
cover a direct outer slot with GROUP BY and an alias without GROUP BY, but not
this cross-product. Please resolve a safe outer-only aggregate-output alias to
its producer while preserving valid aggregate outputs, or reject this shape
explicitly, and add a complete-pipeline grouped-alias regression.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/FillUpQualifyMissingSlot.java:
##########
@@ -189,10 +238,37 @@ interface PlanGenerator {
Plan apply(Set<Expression> newConjuncts, List<NamedExpression>
projects);
}
- private Plan createPlan(LogicalProject<Plan> project, Set<Expression>
conjuncts, PlanGenerator planGenerator) {
+ private Plan createPlan(LogicalProject<Plan> project, Set<Expression>
conjuncts,
+ Optional<Scope> outerScope, PlanGenerator planGenerator) {
Set<Slot> projectOutputSet = project.getOutputSet();
List<NamedExpression> newOutputSlots = Lists.newArrayList();
Set<Expression> newConjuncts = new LinkedHashSet<>();
+
+ // A correlated column referenced in qualify may be hidden behind a
project alias, e.g.
+ // `QUALIFY f = 1` where f is aliased as an outer column o.flag. If
the project also
+ // contains a window expression, filter pushdown cannot rewrite f back
to its producer
+ // before apply decorrelation, so the alias-producer dependency would
be lost and the
+ // correlation slot would never be collected into the apply. Resolve
such aliases whose
+ // producers reference only outer correlated slots, so the correlation
stays visible to
+ // subquery unnesting.
+ Map<Slot, Expression> correlatedAliasToProducer = Maps.newHashMap();
+ if (outerScope.isPresent()) {
+ Set<Slot> correlatedSlots = outerScope.get().getCorrelatedSlots();
+ for (Map.Entry<Slot, Expression> entry :
project.getAliasToProducer().entrySet()) {
+ Expression producer = entry.getValue();
+ if (!producer.getInputSlots().isEmpty()
+ &&
correlatedSlots.containsAll(producer.getInputSlots())) {
+ correlatedAliasToProducer.put(entry.getKey(), producer);
Review Comment:
[P1] Exclude window producers from this alias replacement cycle
A project alias can itself be a window over only outer slots:
```text
Qualify[rn = 1]
Project[row_number() over(order by o.flag) AS rn]
Scan(i)
```
The map replaces `rn` with the window expression, then `visitWindow`
immediately creates a fresh alias `w1`. The generated child is another
`Qualify(Project)` whose `w1` maps to the same window; the top-down analysis
job revisits it, creates `w2`, and repeats without a fixed point before
normalization or Apply unnesting. Restrict this repair to a producer class that
cannot re-enter window extraction (or make the transformation explicitly
idempotent), and add a full analyzer regression with a window partition/order
key that is entirely correlated.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/FillUpQualifyMissingSlot.java:
##########
@@ -189,10 +238,37 @@ interface PlanGenerator {
Plan apply(Set<Expression> newConjuncts, List<NamedExpression>
projects);
}
- private Plan createPlan(LogicalProject<Plan> project, Set<Expression>
conjuncts, PlanGenerator planGenerator) {
+ private Plan createPlan(LogicalProject<Plan> project, Set<Expression>
conjuncts,
+ Optional<Scope> outerScope, PlanGenerator planGenerator) {
Set<Slot> projectOutputSet = project.getOutputSet();
List<NamedExpression> newOutputSlots = Lists.newArrayList();
Set<Expression> newConjuncts = new LinkedHashSet<>();
+
+ // A correlated column referenced in qualify may be hidden behind a
project alias, e.g.
+ // `QUALIFY f = 1` where f is aliased as an outer column o.flag. If
the project also
+ // contains a window expression, filter pushdown cannot rewrite f back
to its producer
+ // before apply decorrelation, so the alias-producer dependency would
be lost and the
+ // correlation slot would never be collected into the apply. Resolve
such aliases whose
+ // producers reference only outer correlated slots, so the correlation
stays visible to
+ // subquery unnesting.
+ Map<Slot, Expression> correlatedAliasToProducer = Maps.newHashMap();
+ if (outerScope.isPresent()) {
+ Set<Slot> correlatedSlots = outerScope.get().getCorrelatedSlots();
+ for (Map.Entry<Slot, Expression> entry :
project.getAliasToProducer().entrySet()) {
+ Expression producer = entry.getValue();
+ if (!producer.getInputSlots().isEmpty()
+ &&
correlatedSlots.containsAll(producer.getInputSlots())) {
+ correlatedAliasToProducer.put(entry.getKey(), producer);
+ }
Review Comment:
[P1] Do not copy a producer subquery into both plan nodes
A producer such as `o.flag + (SELECT max(j.k) FROM j)` has visible input
slots `{o.flag}`, so it enters this map even though it also contains a
`SubqueryExpr`. Replacement leaves the same subquery in the lower project and
copied upper QUALIFY expression:
```text
Qualify[o.flag + scalarSubquery > 0]
Project[o.flag + scalarSubquery AS f, window AS rn]
```
Bottom-up `SubqueryToApply` unnests the lower occurrence and marks that
subquery analyzed. The upper occurrence is then replaced with the same output
slot, but creation of its Apply is skipped by the global analyzed-expression
fence; the intervening project does not output that slot, so final validation
rejects the filter. Exclude producers containing `SubqueryExpr` from this
substitution or materialize/unnest the producer exactly once at a shared level,
with scalar and EXISTS producer regressions through `CheckAfterRewrite`.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushProjectIntoUnion.java:
##########
@@ -100,6 +100,15 @@ private boolean
canPushProjectIntoUnion(LogicalProject<LogicalUnion> project) {
if (union.getQualifier() != Qualifier.ALL || union.arity() != 0) {
return false;
}
+ // The project must only consume slots produced by the union. A
correlated subquery may
+ // still have a project referencing an outer correlated slot (e.g. an
aliased outer column
+ // that is resolved back to its producer); such slots have no constant
producer in the
+ // union, so pushing the project into the union would leave a dangling
slot reference.
+ for (NamedExpression ne : project.getProjects()) {
+ if (!union.getOutputSet().containsAll(ne.getInputSlots())) {
+ return false;
Review Comment:
[P1] Enforce this ownership invariant in the earlier UNION rewrite
This late guard is bypassed by `PushProjectThroughUnion`, which runs before
`MergeOneRowRelationIntoUnion` and this rule in both rewrite pipelines. For a
width-matched project over a constant `UNION ALL`:
```text
Project[i.k, o.flag AS f]
UnionAll[OneRow(...), OneRow(...)]
```
`PushProjectThroughUnion.canPushProject` admits the project from width and
bare/cast-slot shape alone; its replacement leaves `o.flag` unmapped in each
one-row child. `ProjectProcessor` merges those projects and
`MergeOneRowRelationIntoUnion` converts them to arity-zero constant rows, so
this new check never executes and final validation sees a dangling outer slot.
Apply the same child-output containment invariant before
`PushProjectThroughUnion` rewrites the children (including the CTE
registration), and add an end-to-end constant-UNION regression whose normalized
project width matches the UNION width.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/FillUpQualifyMissingSlot.java:
##########
@@ -158,17 +203,21 @@ public List<Rule> buildRules() {
qualify -> having -> project(distinct)
*/
RuleType.FILL_UP_QUALIFY_HAVING_PROJECT.build(
- logicalQualify(logicalHaving(logicalProject())).then(qualify
-> {
+ logicalQualify(logicalHaving(logicalProject())).thenApply(ctx
-> {
+ LogicalQualify<LogicalHaving<LogicalProject<Plan>>>
qualify = ctx.root;
checkWindow(qualify);
+ Optional<Scope> outerScope =
ctx.cascadesContext.getOuterScope();
LogicalHaving<LogicalProject<Plan>> having =
qualify.child();
LogicalProject<Plan> project = qualify.child().child();
- return createPlan(project, qualify.getConjuncts(),
(newConjuncts, projects) -> {
+ return createPlan(project, qualify.getConjuncts(),
outerScope, (newConjuncts, projects) -> {
ImmutableList<NamedExpression> copyOutput =
ImmutableList.copyOf(project.getOutput());
if (project.isDistinct()) {
Set<Slot> missingSlots =
having.getExpressions().stream()
.map(Expression::getInputSlots)
.flatMap(Set::stream)
.filter(s -> !projects.contains(s))
+ .filter(s -> !(outerScope.isPresent()
+ &&
outerScope.get().getCorrelatedSlots().contains(s)))
Review Comment:
[P1] Keep both DISTINCT correlations reachable by the enclosing Apply
For a DISTINCT subquery with separately correlated HAVING and QUALIFY
predicates, this branch builds the reduced tree:
```text
Having[o.h]
ProjectDistinct[i.k]
Qualify[o.flag, rn]
Project[i.k, rn := window]
Scan(i)
```
Distinct conversion plus `NormalizeAggregate` inserts a `Project ->
Aggregate` barrier between the two filters. `UnCorrelatedApplyFilter` first
records `o.h`; the Apply's `alreadyExecutedEliminateFilter` fence then prevents
pulling through the normalization project, and no aggregate-filter rule reaches
the lower `o.flag`. Apply-to-Join therefore uses only `o.h`, leaving `o.flag`
dangling in the right subtree. This is the DISTINCT/project parallel shape, not
the existing `Qualify(Having(Aggregate))` thread. Keep the correlated
predicates on the same decorrelatable side of the distinct/window barriers, or
reject the shape, and add the missing full-pipeline regression.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/FillUpQualifyMissingSlot.java:
##########
@@ -135,21 +142,59 @@ public List<Rule> buildRules() {
qualify -> having -> agg
*/
RuleType.FILL_UP_QUALIFY_HAVING_AGGREGATE.build(
- logicalQualify(logicalHaving(aggregate())).then(qualify -> {
+ logicalQualify(logicalHaving(aggregate())).thenApply(ctx -> {
+ LogicalQualify<LogicalHaving<Aggregate<Plan>>> qualify =
ctx.root;
checkWindow(qualify);
LogicalHaving<Aggregate<Plan>> having = qualify.child();
Aggregate<Plan> agg = qualify.child().child();
- Resolver resolver = new Resolver(agg);
- qualify.getConjuncts().forEach(expr ->
resolver.resolve(expr, ResolvePlanType.QUALIFY));
+ Optional<Scope> outerScope =
ctx.cascadesContext.getOuterScope();
+ // The window expression in qualify will be extracted into
a project above the having
+ // during NormalizeAggregate. Filter pushdown refuses to
go through a project that
+ // contains a window expression, so a correlated predicate
that stays in the having
+ // would sit below the window project and could never be
collected into the apply
+ // during subquery unnesting (it would then be silently
dropped later). Conjoin the
+ // having's outer-only correlated predicates into the
qualify, so both correlated
+ // predicates stay above the window project and are
decorrelated together.
+ Set<Expression> newHavingConjuncts = new LinkedHashSet<>();
+ Set<Expression> qualifyConjuncts = new
LinkedHashSet<>(qualify.getConjuncts());
+ if (outerScope.isPresent()) {
+ Set<Slot> correlatedSlots =
outerScope.get().getCorrelatedSlots();
+ for (Expression conjunct : having.getConjuncts()) {
+ Set<Slot> inputSlots = conjunct.getInputSlots();
+ if (inputSlots.isEmpty()) {
+ newHavingConjuncts.add(conjunct);
+ } else if
(correlatedSlots.containsAll(inputSlots)) {
+ // the predicate only depends on the outer
row, so it can safely be
Review Comment:
[P1] Do not infer HAVING/window commutativity from visible input slots
This containment check is not proof that a conjunct is constant over
aggregate rows or safe to move across window evaluation. For example:
```text
Qualify[rn = 1]
Project[rn := row_number(order by i.k desc)]
Having[count(*) = o.h]
Aggregate[groupBy=i.k, count(*)]
```
`count(*) = o.h` reports only `{o.h}` as input slots, so the patch ranks
every group before applying the predicate. With `o.h=1`, a one-row `k=1` group
and a two-row `k=2` group, correct HAVING-first execution keeps `k=1` and
numbers it 1; the rewrite numbers `k=2` first and returns no row.
`SubqueryExpr` similarly hides a current-group correlation from
`getInputSlots()`, and even a pure false outer predicate may be required to
suppress an error-producing window key. Preserve HAVING below the window while
making it decorrelatable, or conservatively reject shapes whose complete
dependencies, determinism, and evaluation domain are not proven; add aggregate,
nested-subquery, and error-gating regressions.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/FillUpQualifyMissingSlot.java:
##########
@@ -189,10 +238,37 @@ interface PlanGenerator {
Plan apply(Set<Expression> newConjuncts, List<NamedExpression>
projects);
}
- private Plan createPlan(LogicalProject<Plan> project, Set<Expression>
conjuncts, PlanGenerator planGenerator) {
+ private Plan createPlan(LogicalProject<Plan> project, Set<Expression>
conjuncts,
+ Optional<Scope> outerScope, PlanGenerator planGenerator) {
Set<Slot> projectOutputSet = project.getOutputSet();
List<NamedExpression> newOutputSlots = Lists.newArrayList();
Set<Expression> newConjuncts = new LinkedHashSet<>();
+
+ // A correlated column referenced in qualify may be hidden behind a
project alias, e.g.
+ // `QUALIFY f = 1` where f is aliased as an outer column o.flag. If
the project also
+ // contains a window expression, filter pushdown cannot rewrite f back
to its producer
+ // before apply decorrelation, so the alias-producer dependency would
be lost and the
+ // correlation slot would never be collected into the apply. Resolve
such aliases whose
+ // producers reference only outer correlated slots, so the correlation
stays visible to
+ // subquery unnesting.
+ Map<Slot, Expression> correlatedAliasToProducer = Maps.newHashMap();
+ if (outerScope.isPresent()) {
+ Set<Slot> correlatedSlots = outerScope.get().getCorrelatedSlots();
+ for (Map.Entry<Slot, Expression> entry :
project.getAliasToProducer().entrySet()) {
+ Expression producer = entry.getValue();
+ if (!producer.getInputSlots().isEmpty()
+ &&
correlatedSlots.containsAll(producer.getInputSlots())) {
+ correlatedAliasToProducer.put(entry.getKey(), producer);
+ }
+ }
+ }
+ boolean conjunctsRewritten = false;
+ if (!correlatedAliasToProducer.isEmpty()) {
+ Set<Expression> rewrittenConjuncts =
ExpressionUtils.replace(conjuncts, correlatedAliasToProducer);
+ conjunctsRewritten = !rewrittenConjuncts.equals(conjuncts);
Review Comment:
[P1] Preserve slot ownership when replacement crosses IN boundaries
Replacing only the QUALIFY reference does not make the producer legal for
every Apply caller. Two reduced failures remain:
```text
Apply[IN, corr=o.flag]
Scan(o)
Project[o.flag AS f] -> Filter[o.flag=1, window]
Apply[EXISTS, corr=o.flag]
Scan(o)
Apply[IN, compare=o.flag](Project[f, window](Scan(i)), Scan(j))
```
In the first, IN/NOT IN must retain `f` as its build value; the IN-specific
`Project(Filter)` fence preserves the right project, and `InApplyToJoin` leaves
it consuming the left-only `o.flag`. In the second, `ExpressionUtils.replace`
descends into `InSubquery.child()`, then `SubqueryToApply` removes that filter
and stores `o.flag` only in the nested IN Apply's `compareExpr`;
`InApplyToJoin` emits a nested join condition whose slot is produced by neither
child. Both fail final slot validation (mark/OR has the same ownership issue).
Reject unsupported outer-dependent IN outputs/nested compares, or
materialize/pass the value at the correct Apply level, and add IN, NOT IN,
nested-IN, and mark regressions.
--
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]
