Skip to content

Commit 3e6ad0f

Browse files
CalvinKirsCalvinKirs
committed
[fix](fe) Resolve correlated qualified columns before dereference
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: In a correlated subquery, a multipart reference could be interpreted as a nested field of an inner column before checking whether its first part was an outer table alias. This caused scalar inner columns to raise a no-such-field error and complex inner columns to bind silently to the wrong expression. Resolve relation-qualified columns across the current and outer scopes before falling back to column-field dereference. ### Release note Fix incorrect Nereids column binding when an outer table alias conflicts with an inner column name in a correlated subquery. ### Check List (For Author) - Test: Unit Test - ./run-fe-ut.sh --run org.apache.doris.nereids.rules.analysis.TestDereference - Added query_p0 regression coverage for scalar and complex inner columns - Behavior changed: Yes (relation-qualified correlated references now take priority over inner column-field dereference) - Does this need documentation: No
1 parent 0494105 commit 3e6ad0f

3 files changed

Lines changed: 158 additions & 9 deletions

File tree

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

Lines changed: 63 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -309,14 +309,30 @@ public Expression visitElementAt(ElementAt elementAt, ExpressionRewriteContext c
309309
@Override
310310
public Expression visitUnboundSlot(UnboundSlot unboundSlot, ExpressionRewriteContext context) {
311311
Optional<Scope> outerScope = getScope().getOuterScope();
312-
Optional<List<? extends Expression>> boundedOpt = Optional.of(bindSlotByThisScope(unboundSlot));
313-
boolean foundInThisScope = !boundedOpt.get().isEmpty();
312+
List<? extends Expression> bounded = ImmutableList.of();
313+
boolean foundInThisScope = false;
314+
boolean canBindOuterScope = bindSlotInOuterScope && outerScope.isPresent();
315+
316+
// A multipart name can be either a relation-qualified column (t.col) or a nested field
317+
// reference (col.field). In a correlated subquery, try the relation-qualified interpretation
318+
// in both visible scopes first, so an inner column named "t" does not hide an outer alias "t".
319+
if (canBindOuterScope && unboundSlot.getNameParts().size() > 1) {
320+
bounded = bindSlotByRelationQualifier(unboundSlot, getScope());
321+
foundInThisScope = !bounded.isEmpty();
322+
if (!foundInThisScope) {
323+
bounded = bindSlotByRelationQualifier(unboundSlot, outerScope.get());
324+
}
325+
}
326+
327+
if (bounded.isEmpty()) {
328+
bounded = bindSlotByThisScope(unboundSlot);
329+
foundInThisScope = !bounded.isEmpty();
330+
}
314331
// Currently only looking for symbols on the previous level.
315-
if (bindSlotInOuterScope && !foundInThisScope && outerScope.isPresent()) {
316-
boundedOpt = Optional.of(bindSlotByScope(unboundSlot, outerScope.get()));
332+
if (canBindOuterScope && bounded.isEmpty()) {
333+
bounded = bindSlotByScope(unboundSlot, outerScope.get());
317334
}
318335
// it is heavy to deduplicate slots in scope. So we deduplicates bounded here
319-
List<? extends Expression> bounded = boundedOpt.get();
320336
if (bounded.size() > 1) {
321337
bounded = bounded.stream().distinct().collect(Collectors.toList());
322338
}
@@ -1192,12 +1208,37 @@ public List<Expression> bindSlotByScope(UnboundSlot unboundSlot, Scope scope) {
11921208
}
11931209
}
11941210

1211+
/** Bind a multipart slot as a relation-qualified column, without treating its first part as a column. */
1212+
private List<Expression> bindSlotByRelationQualifier(UnboundSlot unboundSlot, Scope scope) {
1213+
List<String> nameParts = unboundSlot.getNameParts();
1214+
Optional<Pair<Integer, Integer>> idxInSql = unboundSlot.getIndexInSqlString();
1215+
switch (nameParts.size()) {
1216+
case 1:
1217+
return ImmutableList.of();
1218+
case 2:
1219+
return (List<Expression>) bindExpressionByTableColumn(
1220+
unboundSlot, nameParts, idxInSql, scope, false);
1221+
case 3:
1222+
return (List<Expression>) bindExpressionByDbTableColumn(
1223+
unboundSlot, nameParts, idxInSql, scope, false);
1224+
default:
1225+
return (List<Expression>) bindExpressionByCatalogDbTableColumn(
1226+
unboundSlot, nameParts, idxInSql, scope, false);
1227+
}
1228+
}
1229+
11951230
private List<? extends Expression> bindExpressionByCatalogDbTableColumn(
11961231
UnboundSlot unboundSlot, List<String> nameParts, Optional<Pair<Integer, Integer>> idxInSql, Scope scope) {
1232+
return bindExpressionByCatalogDbTableColumn(unboundSlot, nameParts, idxInSql, scope, true);
1233+
}
1234+
1235+
private List<? extends Expression> bindExpressionByCatalogDbTableColumn(
1236+
UnboundSlot unboundSlot, List<String> nameParts, Optional<Pair<Integer, Integer>> idxInSql,
1237+
Scope scope, boolean fallbackToColumn) {
11971238
List<Slot> slots = bindSingleSlotByCatalog(
11981239
nameParts.get(0), nameParts.get(1), nameParts.get(2), nameParts.get(3), scope);
11991240
if (slots.isEmpty()) {
1200-
return bindExpressionByDbTableColumn(unboundSlot, nameParts, idxInSql, scope);
1241+
return bindExpressionByDbTableColumn(unboundSlot, nameParts, idxInSql, scope, fallbackToColumn);
12011242
} else if (slots.size() > 1) {
12021243
return addSqlIndexInfo(slots, idxInSql);
12031244
}
@@ -1216,9 +1257,15 @@ private List<? extends Expression> bindExpressionByCatalogDbTableColumn(
12161257

12171258
private List<? extends Expression> bindExpressionByDbTableColumn(
12181259
UnboundSlot unboundSlot, List<String> nameParts, Optional<Pair<Integer, Integer>> idxInSql, Scope scope) {
1260+
return bindExpressionByDbTableColumn(unboundSlot, nameParts, idxInSql, scope, true);
1261+
}
1262+
1263+
private List<? extends Expression> bindExpressionByDbTableColumn(
1264+
UnboundSlot unboundSlot, List<String> nameParts, Optional<Pair<Integer, Integer>> idxInSql,
1265+
Scope scope, boolean fallbackToColumn) {
12191266
List<Slot> slots = bindSingleSlotByDb(nameParts.get(0), nameParts.get(1), nameParts.get(2), scope);
12201267
if (slots.isEmpty()) {
1221-
return bindExpressionByTableColumn(unboundSlot, nameParts, idxInSql, scope);
1268+
return bindExpressionByTableColumn(unboundSlot, nameParts, idxInSql, scope, fallbackToColumn);
12221269
} else if (slots.size() > 1) {
12231270
return addSqlIndexInfo(slots, idxInSql);
12241271
}
@@ -1237,9 +1284,17 @@ private List<? extends Expression> bindExpressionByDbTableColumn(
12371284

12381285
private List<? extends Expression> bindExpressionByTableColumn(
12391286
UnboundSlot unboundSlot, List<String> nameParts, Optional<Pair<Integer, Integer>> idxInSql, Scope scope) {
1287+
return bindExpressionByTableColumn(unboundSlot, nameParts, idxInSql, scope, true);
1288+
}
1289+
1290+
private List<? extends Expression> bindExpressionByTableColumn(
1291+
UnboundSlot unboundSlot, List<String> nameParts, Optional<Pair<Integer, Integer>> idxInSql,
1292+
Scope scope, boolean fallbackToColumn) {
12401293
List<Slot> slots = bindSingleSlotByTable(nameParts.get(0), nameParts.get(1), scope);
12411294
if (slots.isEmpty()) {
1242-
return bindExpressionByColumn(unboundSlot, nameParts, idxInSql, scope);
1295+
return fallbackToColumn
1296+
? bindExpressionByColumn(unboundSlot, nameParts, idxInSql, scope)
1297+
: ImmutableList.of();
12431298
} else if (slots.size() > 1) {
12441299
return addSqlIndexInfo(slots, idxInSql);
12451300
}

‎fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/TestDereference.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,14 @@
2222
import org.apache.doris.catalog.VariantType;
2323
import org.apache.doris.common.FeConstants;
2424
import org.apache.doris.datasource.test.TestExternalCatalog.TestCatalogProvider;
25+
import org.apache.doris.nereids.trees.plans.Plan;
26+
import org.apache.doris.nereids.trees.plans.logical.LogicalApply;
2527
import org.apache.doris.nereids.util.PlanChecker;
2628
import org.apache.doris.utframe.TestWithFeService;
2729

2830
import com.google.common.collect.ImmutableList;
2931
import com.google.common.collect.ImmutableMap;
32+
import org.junit.jupiter.api.Assertions;
3033
import org.junit.jupiter.api.Test;
3134

3235
import java.util.List;
@@ -39,6 +42,19 @@ public class TestDereference extends TestWithFeService {
3942
"t", ImmutableList.of(
4043
new Column("id", PrimitiveType.INT),
4144
new Column("t", new VariantType())
45+
),
46+
"outer_table", ImmutableList.of(
47+
new Column("id", PrimitiveType.INT),
48+
new Column("value", PrimitiveType.INT),
49+
new Column("@event_name", PrimitiveType.VARCHAR)
50+
),
51+
"inner_table", ImmutableList.of(
52+
new Column("id", PrimitiveType.INT),
53+
new Column("t1", PrimitiveType.INT)
54+
),
55+
"inner_variant_table", ImmutableList.of(
56+
new Column("id", PrimitiveType.INT),
57+
new Column("outer_alias", new VariantType())
4258
)
4359
)
4460
);
@@ -70,6 +86,27 @@ public void testBindPriority() {
7086
testBind("select t.t.t.t.t.t from t");
7187
}
7288

89+
@Test
90+
public void testCorrelatedSubqueryPrefersOuterTableAlias() {
91+
testBind("select t1.`@event_name` from outer_table t1 where exists ("
92+
+ "select 1 from inner_table inner_alias where t1.`@event_name` = 'click')");
93+
}
94+
95+
@Test
96+
public void testOuterTableAliasTakesPriorityOverInnerVariantColumn() {
97+
Plan plan = PlanChecker.from(connectContext)
98+
.analyze("select outer_alias.id from outer_table outer_alias where exists ("
99+
+ "select 1 from inner_variant_table inner_alias where outer_alias.value = 1)")
100+
.getPlan();
101+
102+
List<LogicalApply<?, ?>> applies = plan.collectToList(LogicalApply.class::isInstance);
103+
Assertions.assertEquals(1, applies.size());
104+
Assertions.assertEquals(1, applies.get(0).getCorrelationSlot().size());
105+
Assertions.assertEquals("value", applies.get(0).getCorrelationSlot().get(0).getName());
106+
List<String> qualifier = applies.get(0).getCorrelationSlot().get(0).getQualifier();
107+
Assertions.assertEquals("outer_alias", qualifier.get(qualifier.size() - 1));
108+
}
109+
73110
private void testBind(String sql) {
74111
PlanChecker.from(connectContext)
75112
.analyze(sql)

‎regression-test/suites/query_p0/test_dereference.groovy‎

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,61 @@ suite("test_dereference") {
6666
sql "select s.a from test_dereference2"
6767
exception "No such struct field 'a' in 's'"
6868
}
69-
}
69+
70+
multi_sql """
71+
drop table if exists test_correlated_dereference_outer;
72+
drop table if exists test_correlated_dereference_inner_scalar;
73+
drop table if exists test_correlated_dereference_inner_struct;
74+
create table test_correlated_dereference_outer(
75+
id int,
76+
value int,
77+
`@event_name` varchar(32)
78+
)
79+
distributed by hash(id)
80+
properties('replication_num'='1');
81+
82+
create table test_correlated_dereference_inner_scalar(
83+
id int,
84+
t1 int
85+
)
86+
distributed by hash(id)
87+
properties('replication_num'='1');
88+
89+
create table test_correlated_dereference_inner_struct(
90+
id int,
91+
outer_alias struct<value:int>
92+
)
93+
distributed by hash(id)
94+
properties('replication_num'='1');
95+
96+
insert into test_correlated_dereference_outer values (1, 10, 'blocked'), (2, 20, 'kept');
97+
insert into test_correlated_dereference_inner_scalar values (1, 0);
98+
insert into test_correlated_dereference_inner_struct values (1, struct(10));
99+
"""
100+
101+
test {
102+
sql """
103+
select t1.id, t1.`@event_name`
104+
from test_correlated_dereference_outer t1
105+
where not exists (
106+
select 1 from test_correlated_dereference_inner_scalar inner_alias
107+
where t1.`@event_name` = 'blocked'
108+
)
109+
order by t1.id
110+
"""
111+
result([[2, 'kept']])
112+
}
113+
114+
test {
115+
sql """
116+
select outer_alias.id, outer_alias.value
117+
from test_correlated_dereference_outer outer_alias
118+
where not exists (
119+
select 1 from test_correlated_dereference_inner_struct inner_alias
120+
where outer_alias.value = 10
121+
)
122+
order by outer_alias.id
123+
"""
124+
result([[2, 20]])
125+
}
126+
}

0 commit comments

Comments
 (0)