Skip to content

Commit ef2a7d2

Browse files
committed
[fix](fe) Fix correlated outer column in QUALIFY being mis-handled after GROUP BY / over project
fix comments fix comments fix fix comment fix comment fix comment
1 parent 3ba2964 commit ef2a7d2

6 files changed

Lines changed: 1143 additions & 21 deletions

File tree

fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/FillUpQualifyMissingSlot.java

Lines changed: 303 additions & 21 deletions
Large diffs are not rendered by default.

fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushProjectIntoUnion.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,15 @@ private boolean canPushProjectIntoUnion(LogicalProject<LogicalUnion> project) {
100100
if (union.getQualifier() != Qualifier.ALL || union.arity() != 0) {
101101
return false;
102102
}
103+
// The project must only consume slots produced by the union. A correlated subquery may
104+
// still have a project referencing an outer correlated slot (e.g. an aliased outer column
105+
// that is resolved back to its producer); such slots have no constant producer in the
106+
// union, so pushing the project into the union would leave a dangling slot reference.
107+
for (NamedExpression ne : project.getProjects()) {
108+
if (!union.getOutputSet().containsAll(ne.getInputSlots())) {
109+
return false;
110+
}
111+
}
103112
for (List<NamedExpression> constExprs : union.getConstantExprsList()) {
104113
Set<Slot> uniqueFunctionSlots = Sets.newHashSet();
105114
for (int i = 0; i < constExprs.size(); i++) {

fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushProjectThroughUnion.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,16 @@ public static boolean canPushProject(List<NamedExpression> projects, LogicalSetO
6363
if (projects.size() != logicalSetOperation.getOutput().size()) {
6464
return false;
6565
}
66+
// A correlated subquery may reference outer slots in its project (e.g. an aliased outer
67+
// column). Such slots have no producer inside the union children, so pushing the project
68+
// through the union would leave a dangling slot reference in each child. Reject those
69+
// projects here, before the children are rewritten (PushProjectIntoUnion has a similar
70+
// late guard, but it is bypassed because this rule runs first).
71+
for (NamedExpression project : projects) {
72+
if (!logicalSetOperation.getOutputSet().containsAll(project.getInputSlots())) {
73+
return false;
74+
}
75+
}
6676
boolean isAll = logicalSetOperation.getQualifier().equals(Qualifier.ALL);
6777
Set<ExprId> projectInputExprIds = Sets.newHashSetWithExpectedSize(projects.size());
6878
for (NamedExpression project : projects) {
Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.apache.doris.nereids.rules.analysis;
19+
20+
import org.apache.doris.nereids.exceptions.AnalysisException;
21+
import org.apache.doris.nereids.rules.rewrite.PullUpProjectUnderApply;
22+
import org.apache.doris.nereids.rules.rewrite.UnCorrelatedApplyFilter;
23+
import org.apache.doris.nereids.trees.expressions.Expression;
24+
import org.apache.doris.nereids.trees.expressions.Slot;
25+
import org.apache.doris.nereids.trees.plans.Plan;
26+
import org.apache.doris.nereids.trees.plans.logical.LogicalApply;
27+
import org.apache.doris.nereids.util.ExpressionUtils;
28+
import org.apache.doris.nereids.util.MemoPatternMatchSupported;
29+
import org.apache.doris.nereids.util.PlanChecker;
30+
import org.apache.doris.utframe.TestWithFeService;
31+
32+
import org.junit.jupiter.api.Assertions;
33+
import org.junit.jupiter.api.Test;
34+
35+
import java.util.List;
36+
import java.util.Optional;
37+
import java.util.Set;
38+
import java.util.stream.Collectors;
39+
40+
public class FillUpQualifyMissingSlotTest extends TestWithFeService implements MemoPatternMatchSupported {
41+
42+
@Override
43+
protected void runBeforeAll() throws Exception {
44+
createDatabase("test");
45+
connectContext.setDatabase("test");
46+
47+
createTables(
48+
"CREATE TABLE test.o (\n"
49+
+ " k INT,\n"
50+
+ " flag INT,\n"
51+
+ " h INT\n"
52+
+ ")\n"
53+
+ "DUPLICATE KEY (k)\n"
54+
+ "DISTRIBUTED BY HASH (k) BUCKETS 3\n"
55+
+ "PROPERTIES(\n"
56+
+ " 'replication_num' = '1'\n"
57+
+ ");",
58+
"CREATE TABLE test.i (\n"
59+
+ " k INT,\n"
60+
+ " not_grouped INT\n"
61+
+ ")\n"
62+
+ "DUPLICATE KEY (k)\n"
63+
+ "DISTRIBUTED BY HASH (k) BUCKETS 3\n"
64+
+ "PROPERTIES(\n"
65+
+ " 'replication_num' = '1'\n"
66+
+ ");"
67+
);
68+
}
69+
70+
private LogicalApply findApply(Plan plan) {
71+
List<LogicalApply> applies = plan.collectToList(LogicalApply.class::isInstance)
72+
.stream().map(node -> (LogicalApply) node).collect(Collectors.toList());
73+
Assertions.assertEquals(1, applies.size(),
74+
"expected exactly one LogicalApply in plan:\n" + plan.treeString());
75+
return applies.get(0);
76+
}
77+
78+
private Set<Slot> getCorrelationFilterInputSlots(LogicalApply apply) {
79+
Optional<Expression> filter = apply.getCorrelationFilter();
80+
Assertions.assertTrue(filter.isPresent(),
81+
"apply should record a correlation filter, plan:\n" + apply.treeString());
82+
Set<Expression> conjuncts = ExpressionUtils.extractConjunctionToSet(filter.get());
83+
return conjuncts.stream().flatMap(e -> e.getInputSlots().stream()).collect(Collectors.toSet());
84+
}
85+
86+
private boolean containsSlotNamed(Set<Slot> slots, String name) {
87+
return slots.stream().anyMatch(s -> s.getName().equals(name));
88+
}
89+
90+
/**
91+
* qualify -> having -> agg where both the having and the qualify reference correlated outer
92+
* columns. The window expression in qualify is extracted into a project above the having during
93+
* NormalizeAggregate; the having's correlated predicate must be conjoined into the qualify so it
94+
* stays above that window project and is still collected into the apply during unnesting.
95+
*/
96+
@Test
97+
public void testCorrelatedQualifyAndHaving() {
98+
String sql = "SELECT o.k\n"
99+
+ "FROM o\n"
100+
+ "WHERE EXISTS (\n"
101+
+ " SELECT i.k\n"
102+
+ " FROM i\n"
103+
+ " GROUP BY i.k\n"
104+
+ " HAVING o.h = 1\n"
105+
+ " QUALIFY row_number() OVER (ORDER BY i.k) = 1 AND o.flag = 1\n"
106+
+ ")\n"
107+
+ "ORDER BY o.k";
108+
Plan plan = PlanChecker.from(connectContext)
109+
.analyze(sql)
110+
.applyBottomUp(new PullUpProjectUnderApply())
111+
.applyBottomUp(new UnCorrelatedApplyFilter())
112+
.getPlan();
113+
LogicalApply apply = findApply(plan);
114+
Set<Slot> slots = getCorrelationFilterInputSlots(apply);
115+
// both the qualify correlation (o.flag) and the having correlation (o.h) must be collected
116+
Assertions.assertTrue(containsSlotNamed(slots, "flag"),
117+
"correlation filter should reference o.flag, plan:\n" + apply.treeString());
118+
Assertions.assertTrue(containsSlotNamed(slots, "h"),
119+
"correlation filter should reference o.h, plan:\n" + apply.treeString());
120+
}
121+
122+
/**
123+
* qualify -> project where the qualify references a project alias (f) whose producer is a
124+
* correlated outer column (o.flag). The alias-producer dependency must be resolved so the
125+
* correlation slot is still collected into the apply even though the window expression in the
126+
* project blocks filter pushdown.
127+
*/
128+
@Test
129+
public void testCorrelatedQualifyWithAlias() {
130+
String sql = "SELECT o.k\n"
131+
+ "FROM o\n"
132+
+ "WHERE EXISTS (\n"
133+
+ " SELECT i.k, o.flag AS f, row_number() OVER (ORDER BY i.k) AS rn\n"
134+
+ " FROM i\n"
135+
+ " QUALIFY rn = 1 AND f = 1\n"
136+
+ ")\n"
137+
+ "ORDER BY o.k";
138+
Plan plan = PlanChecker.from(connectContext)
139+
.analyze(sql)
140+
.applyBottomUp(new PullUpProjectUnderApply())
141+
.applyBottomUp(new UnCorrelatedApplyFilter())
142+
.getPlan();
143+
LogicalApply apply = findApply(plan);
144+
Set<Slot> slots = getCorrelationFilterInputSlots(apply);
145+
Assertions.assertTrue(containsSlotNamed(slots, "flag"),
146+
"correlation filter should reference o.flag (resolved from alias f), plan:\n"
147+
+ apply.treeString());
148+
}
149+
150+
/**
151+
* A having predicate that mixes outer correlated slots with aggregate results cannot be
152+
* moved above the window project (it depends on the aggregate rows), and the window project
153+
* would prevent subquery unnesting from collecting its correlation. Such a shape must be
154+
* rejected during analysis instead of silently dropping the predicate.
155+
*/
156+
@Test
157+
public void testMixedCorrelatedHavingRejected() {
158+
String sql = "SELECT o.k\n"
159+
+ "FROM o\n"
160+
+ "WHERE EXISTS (\n"
161+
+ " SELECT i.k\n"
162+
+ " FROM i\n"
163+
+ " GROUP BY i.k\n"
164+
+ " HAVING o.h = sum(i.k)\n"
165+
+ " QUALIFY row_number() OVER (ORDER BY i.k) = 1 AND o.flag = 1\n"
166+
+ ")\n"
167+
+ "ORDER BY o.k";
168+
AnalysisException exception = Assertions.assertThrows(AnalysisException.class,
169+
() -> PlanChecker.from(connectContext).analyze(sql));
170+
Assertions.assertTrue(exception.getMessage().contains("not supported"),
171+
"unexpected exception message: " + exception.getMessage());
172+
}
173+
}

regression-test/data/query_p0/sql_functions/window_functions/test_qualify_query.out

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,3 +121,46 @@ Finland Phone 11
121121
-- !select_36 --
122122
2001 Finland 6
123123

124+
-- !select_37 --
125+
10
126+
127+
-- !select_38 --
128+
10
129+
130+
-- !select_39 --
131+
10
132+
133+
-- !select_40 --
134+
135+
-- !select_41 --
136+
10
137+
138+
-- !select_42 --
139+
10
140+
141+
-- !select_43 --
142+
143+
-- !select_44 --
144+
10
145+
146+
-- !select_45 --
147+
10
148+
149+
-- !select_46 --
150+
10
151+
152+
-- !select_47 --
153+
10
154+
155+
-- !select_48 --
156+
10
157+
158+
-- !select_49 --
159+
10
160+
161+
-- !select_50 --
162+
10
163+
164+
-- !select_51 --
165+
10
166+

0 commit comments

Comments
 (0)