Skip to content

Commit a369458

Browse files
authored
Merge pull request #3172 from dolthub/angela/any_scope
Add correlated columns from subqueries to GroupBy select dependencies
2 parents 3d7fa0e + d238ea5 commit a369458

File tree

4 files changed

+45
-9
lines changed

4 files changed

+45
-9
lines changed

enginetest/queries/queries.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9374,6 +9374,24 @@ from typestable`,
93749374
Query: "select c1, c2 from one_pk where c2 = 1 group by c1",
93759375
Expected: []sql.Row{{0, 1}},
93769376
},
9377+
// https://github.com/dolthub/dolt/issues/9699
9378+
// Correlated columns in subqueries are included in select dependencies
9379+
{
9380+
Query: "select any_value(pk), (select max(pk) from one_pk where pk < opk.pk) as x from one_pk opk",
9381+
Expected: []sql.Row{{0, nil}, {1, 0}, {2, 1}, {3, 2}},
9382+
},
9383+
{
9384+
Query: "SELECT any_value(pk), (SELECT max(pk) FROM one_pk WHERE pk < opk.pk) AS x FROM one_pk opk WHERE (SELECT max(pk) FROM one_pk WHERE pk < opk.pk) > 0;",
9385+
Expected: []sql.Row{{2, 1}, {3, 2}},
9386+
},
9387+
{
9388+
Query: "select any_value(pk), (select max(pk) from one_pk where pk < (opk.c1 - 10)) as x from one_pk opk",
9389+
Expected: []sql.Row{{0, nil}, {1, nil}, {2, 3}, {3, 3}},
9390+
},
9391+
{
9392+
Query: "select pk, (select max(pk) from one_pk where pk < opk.pk) as x from one_pk opk",
9393+
Expected: []sql.Row{{0, nil}, {1, 0}, {2, 1}, {3, 2}},
9394+
},
93779395
}
93789396

93799397
var KeylessQueries = []QueryTest{

sql/planbuilder/aggregates.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -197,9 +197,7 @@ func (b *Builder) buildAggregation(fromScope, projScope *scope, groupingCols []s
197197

198198
group := fromScope.groupBy
199199
outScope := group.outScope
200-
// select columns:
201-
// - aggs
202-
// - extra columns needed by having, order by, select
200+
// Select dependencies include aggregations and table columns needed for projections, having, and sort (order by)
203201
var selectDeps []sql.Expression
204202
var selectGfs []sql.Expression
205203
selectStr := make(map[string]bool)
@@ -224,8 +222,8 @@ func (b *Builder) buildAggregation(fromScope, projScope *scope, groupingCols []s
224222
default:
225223
}
226224

227-
// projection dependencies -> table cols needed above
228-
transform.InspectExpr(col.scalar, func(e sql.Expression) bool {
225+
var findSelectDeps func(sql.Expression) bool
226+
findSelectDeps = func(e sql.Expression) bool {
229227
switch e := e.(type) {
230228
case *expression.GetField:
231229
colName := strings.ToLower(e.String())
@@ -241,10 +239,18 @@ func (b *Builder) buildAggregation(fromScope, projScope *scope, groupingCols []s
241239
} else if isAliasDep && !inAlias {
242240
aliasDeps[exprStr] = false
243241
}
242+
case *plan.Subquery:
243+
e.Correlated().ForEach(func(colId sql.ColumnId) {
244+
if correlated, found := projScope.parent.getCol(colId); found {
245+
findSelectDeps(correlated.scalarGf())
246+
}
247+
})
244248
default:
245249
}
246250
return false
247-
})
251+
}
252+
253+
transform.InspectExpr(col.scalar, findSelectDeps)
248254
}
249255
for _, e := range fromScope.extraCols {
250256
// accessory cols used by ORDER_BY, HAVING

sql/planbuilder/parse_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1058,7 +1058,7 @@ Project
10581058
│ └─ tableId: 2
10591059
│ ->(select u from uv where x = u)]
10601060
└─ GroupBy
1061-
├─ select:
1061+
├─ select: xy.x:1!null
10621062
├─ group: Subquery
10631063
│ ├─ cacheable: false
10641064
│ ├─ alias-string: select u from uv where x = u
@@ -2050,7 +2050,7 @@ Project
20502050
│ └─ tableId: 0
20512051
│ ->a1:8]
20522052
└─ Project
2053-
├─ columns: [max(xy.x):4!null, Subquery
2053+
├─ columns: [max(xy.x):4!null, xy.x:1!null, Subquery
20542054
│ ├─ cacheable: false
20552055
│ ├─ alias-string: select max(dt.a) from (select x as a) as dt (a)
20562056
│ └─ Project
@@ -2074,7 +2074,7 @@ Project
20742074
│ └─ tableId: 0
20752075
│ ->a1:8]
20762076
└─ GroupBy
2077-
├─ select: MAX(xy.x:1!null)
2077+
├─ select: MAX(xy.x:1!null), xy.x:1!null
20782078
├─ group: Subquery
20792079
│ ├─ cacheable: false
20802080
│ ├─ alias-string: select max(dt.a) from (select x as a) as dt (a)

sql/planbuilder/scope.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,18 @@ func (s *scope) resolveColumn(db, table, col string, checkParent, chooseFirst bo
149149
return c, true
150150
}
151151

152+
// getCol gets a scopeColumn based on a columnId
153+
func (s *scope) getCol(colId sql.ColumnId) (scopeColumn, bool) {
154+
if s.colset.Contains(colId) {
155+
for _, c := range s.cols {
156+
if sql.ColumnId(c.id) == colId {
157+
return c, true
158+
}
159+
}
160+
}
161+
return scopeColumn{}, false
162+
}
163+
152164
func (s *scope) hasTable(table string) bool {
153165
_, ok := s.tables[strings.ToLower(table)]
154166
if ok {

0 commit comments

Comments
 (0)