Skip to content

Commit

Permalink
resolve unqualified columns in analyzer by looking in parent scope if…
Browse files Browse the repository at this point in the history
… on current scope it is not able to resolve

Signed-off-by: Harshit Gangal <[email protected]>
  • Loading branch information
harshit-gangal committed Jul 22, 2021
1 parent 1e8b3bd commit 9b54895
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 35 deletions.
2 changes: 1 addition & 1 deletion go/vt/vtgate/planbuilder/route_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ type horizonPlanning struct {
vtgateGrouping bool
}

func (hp horizonPlanning) planHorizon() (logicalPlan, error) {
func (hp *horizonPlanning) planHorizon() (logicalPlan, error) {
rb, ok := hp.plan.(*route)
if !ok && hp.semTable.ProjectionErr != nil {
return nil, hp.semTable.ProjectionErr
Expand Down
1 change: 1 addition & 0 deletions go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ Gen4 error: unsupported: in scatter query: complex aggregate expression
# group by and ',' joins
"select user.id from user, user_extra group by id"
"unsupported: cross-shard query with aggregates"
Gen4 error: Expression of SELECT list is not in GROUP BY clause and contains nonaggregated column '`user`.id' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by

# if subquery scatter and ordering, then we don't allow outer constructs to be pushed down.
"select count(*) from (select col, user_extra.extra from user join user_extra on user.id = user_extra.user_id order by user_extra.extra) a"
Expand Down
6 changes: 6 additions & 0 deletions go/vt/vtgate/semantics/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,8 @@ func (a *analyzer) depsForExpr(expr sqlparser.Expr) TableSet {
// resolveUnQualifiedColumn
func (a *analyzer) resolveUnQualifiedColumn(current *scope, expr *sqlparser.ColName) (TableSet, error) {
var tsp *TableSet

tryAgain:
for _, tbl := range current.tables {
ts := tbl.DepsFor(expr, a, len(current.tables) == 1)
if ts != nil && tsp != nil {
Expand All @@ -296,6 +298,10 @@ func (a *analyzer) resolveUnQualifiedColumn(current *scope, expr *sqlparser.ColN
tsp = ts
}
}
if tsp == nil && current.parent != nil {
current = current.parent
goto tryAgain
}
if tsp == nil {
return 0, nil
}
Expand Down
93 changes: 59 additions & 34 deletions go/vt/vtgate/semantics/analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,39 +144,49 @@ func TestOrderByBindingSingleTable(t *testing.T) {
}

func TestGroupByBindingSingleTable(t *testing.T) {
t.Run("positive tests", func(t *testing.T) {
tcases := []struct {
sql string
deps TableSet
}{{
"select col from tabl group by col",
T1,
}, {
"select col from tabl group by tabl.col",
T1,
}, {
"select col from tabl group by d.tabl.col",
T1,
}, {
"select col from tabl group by 1",
T1,
}, {
"select col as c from tabl group by c",
T1,
}, {
"select 1 as c from tabl group by c",
T0,
}}
for _, tc := range tcases {
t.Run(tc.sql, func(t *testing.T) {
stmt, semTable := parseAndAnalyze(t, tc.sql, "d")
sel, _ := stmt.(*sqlparser.Select)
grp := sel.GroupBy[0]
d := semTable.Dependencies(grp)
require.Equal(t, tc.deps, d, tc.sql)
})
}
})
tcases := []struct {
sql string
deps TableSet
}{{
"select col from tabl group by col",
T1,
}, {
"select col from tabl group by tabl.col",
T1,
}, {
"select col from tabl group by d.tabl.col",
T1,
}, {
"select tabl.col as x from tabl group by x",
T1,
}, {
"select tabl.col as x from tabl group by col",
T1,
}, {
"select col from tabl group by 1",
T1,
}, {
"select col as c from tabl group by c",
T1,
}, {
"select 1 as c from tabl group by c",
T0,
}, {
"select t1.id from t1, t2 group by id",
T1,
}, {
"select t.id from t, t1 group by id",
T2,
}}
for _, tc := range tcases {
t.Run(tc.sql, func(t *testing.T) {
stmt, semTable := parseAndAnalyze(t, tc.sql, "d")
sel, _ := stmt.(*sqlparser.Select)
grp := sel.GroupBy[0]
d := semTable.Dependencies(grp)
require.Equal(t, tc.deps, d, tc.sql)
})
}
}

func TestBindingSingleAliasedTable(t *testing.T) {
Expand Down Expand Up @@ -520,9 +530,24 @@ func parseAndAnalyze(t *testing.T, query, dbName string) (sqlparser.Statement, *
t.Helper()
parse, err := sqlparser.Parse(query)
require.NoError(t, err)

cols1 := []vindexes.Column{{
Name: sqlparser.NewColIdent("id"),
Type: querypb.Type_INT64,
}}
cols2 := []vindexes.Column{{
Name: sqlparser.NewColIdent("uid"),
Type: querypb.Type_INT64,
}, {
Name: sqlparser.NewColIdent("name"),
Type: querypb.Type_VARCHAR,
}}

semTable, err := Analyze(parse, dbName, &FakeSI{
Tables: map[string]*vindexes.Table{
"t": {Name: sqlparser.NewTableIdent("t")},
"t": {Name: sqlparser.NewTableIdent("t")},
"t1": {Name: sqlparser.NewTableIdent("t1"), Columns: cols1, ColumnListAuthoritative: true},
"t2": {Name: sqlparser.NewTableIdent("t2"), Columns: cols2, ColumnListAuthoritative: true},
},
})
require.NoError(t, err)
Expand Down

0 comments on commit 9b54895

Please sign in to comment.