Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Gen4] Implemented table alias functionality in the semantic analysis #7629

Merged
merged 10 commits into from
Apr 27, 2021
4 changes: 2 additions & 2 deletions go/vt/vtgate/planbuilder/querygraph_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func TestQueryGraph(t *testing.T) {
t.Run(fmt.Sprintf("%d %s", i, sql), func(t *testing.T) {
tree, err := sqlparser.Parse(sql)
require.NoError(t, err)
semTable, err := semantics.Analyse(tree, &fakeSI{})
semTable, err := semantics.Analyze(tree, "", &fakeSI{})
require.NoError(t, err)
qgraph, err := createQGFromSelect(tree.(*sqlparser.Select), semTable)
require.NoError(t, err)
Expand All @@ -136,7 +136,7 @@ func TestQueryGraph(t *testing.T) {
func TestString(t *testing.T) {
tree, err := sqlparser.Parse("select * from a,b join c on b.id = c.id where a.id = b.id and b.col IN (select 42) and func() = 'foo'")
require.NoError(t, err)
semTable, err := semantics.Analyse(tree, &fakeSI{})
semTable, err := semantics.Analyze(tree, "", &fakeSI{})
require.NoError(t, err)
qgraph, err := createQGFromSelect(tree.(*sqlparser.Select), semTable)
require.NoError(t, err)
Expand Down
20 changes: 18 additions & 2 deletions go/vt/vtgate/planbuilder/route_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@ func gen4Planner(_ string) func(sqlparser.Statement, *sqlparser.ReservedVars, Co
}

func newBuildSelectPlan(sel *sqlparser.Select, vschema ContextVSchema) (engine.Primitive, error) {
semTable, err := semantics.Analyse(sel, vschema)
keyspace, err := vschema.DefaultKeyspace()
if err != nil {
return nil, err
}
Comment on lines +49 to +52
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will start failing for cases when there is no default database selected.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm... how come no tests are failing? do we need to add more tests to the planner?

you are probably absolutely correct about this comment. that being said - since this is not failing any tests, how about we take care of this in a separate PR? We should add more tests, and then also fix the issue

WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are introducing this check here, so it be be better to have a test that covers the error case if existing tests do not fail.

semTable, err := semantics.Analyze(sel, keyspace.Name, vschema)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -645,7 +649,19 @@ func createRoutePlan(table *queryTable, solves semantics.TableSet, vschema Conte
return nil, err
}
if vschemaTable.Name.String() != table.table.Name.String() {
return nil, semantics.Gen4NotSupportedF("routed tables")
// we are dealing with a routed table
name := table.table.Name
table.table.Name = vschemaTable.Name
astTable, ok := table.alias.Expr.(sqlparser.TableName)
if !ok {
return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "[BUG] a derived table should never be a routed table")
}
realTableName := sqlparser.NewTableIdent(vschemaTable.Name.String())
astTable.Name = realTableName
if table.alias.As.IsEmpty() {
// if the user hasn't specified an alias, we'll insert one here so the old table name still works
table.alias.As = sqlparser.NewTableIdent(name.String())
}
}
plan := &routePlan{
solved: solves,
Expand Down
1 change: 1 addition & 0 deletions go/vt/vtgate/planbuilder/testdata/aggr_cases.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1244,6 +1244,7 @@ Gen4 plan same as above
"Table": "unsharded"
}
}
Gen4 plan same as above

# order by on a reference table
"select col from ref order by col"
Expand Down
1 change: 1 addition & 0 deletions go/vt/vtgate/planbuilder/testdata/filter_cases.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1034,6 +1034,7 @@ Gen4 plan same as above
"Vindex": "user_index"
}
}
Gen4 plan same as above

# subquery of information_schema with itself
"select * from information_schema.a where id in (select * from information_schema.b)"
Expand Down
3 changes: 3 additions & 0 deletions go/vt/vtgate/planbuilder/testdata/from_cases.txt
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ Gen4 plan same as above
"Table": "unsharded"
}
}
Gen4 plan same as above

# ',' join information_schema
"select a.id,b.id from information_schema.a as a, information_schema.b as b"
Expand Down Expand Up @@ -337,6 +338,7 @@ Gen4 plan same as above
"Table": "unsharded"
}
}
Gen4 plan same as above

# Left join, single chunk
"select m1.col from unsharded as m1 left join unsharded as m2 on m1.a=m2.b"
Expand Down Expand Up @@ -1247,6 +1249,7 @@ Gen4 plan same as above
]
}
}
Gen4 plan same as above

# subquery
"select id from (select id, col from user where id = 5) as t"
Expand Down
1 change: 1 addition & 0 deletions go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,7 @@ Gen4 plan same as above
"Vindex": "user_index"
}
}
Gen4 plan same as above

# LIMIT
"select col1 from user where id = 1 limit 1"
Expand Down
3 changes: 3 additions & 0 deletions go/vt/vtgate/planbuilder/testdata/select_cases.txt
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,7 @@ Gen4 plan same as above
"Vindex": "user_index"
}
}
Gen4 plan same as above

# Column Aliasing with Column
"select user0_.col as col0_ from user user0_ where id = 1 order by col0_ desc limit 3"
Expand All @@ -969,6 +970,7 @@ Gen4 plan same as above
"Vindex": "user_index"
}
}
Gen4 plan same as above

# Booleans and parenthesis
"select * from user where (id = 1) AND name = true limit 5"
Expand Down Expand Up @@ -1350,6 +1352,7 @@ Gen4 plan same as above
"Table": "unsharded"
}
}
Gen4 plan same as above

# testing SingleRow Projection
"select 42"
Expand Down
2 changes: 2 additions & 0 deletions go/vt/vtgate/planbuilder/testdata/wireup_cases.txt
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,9 @@
# Invalid value in IN clause from LHS of join
"select u1.id from user u1 join user u2 where u1.id = 18446744073709551616"
"strconv.ParseUint: parsing "18446744073709551616": value out of range"
Gen4 plan same as above

# Invalid value in IN clause from RHS of join
"select u1.id from user u1 join user u2 where u2.id = 18446744073709551616"
"strconv.ParseUint: parsing "18446744073709551616": value out of range"
Gen4 plan same as above
75 changes: 50 additions & 25 deletions go/vt/vtgate/semantics/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,32 @@ type (
analyzer struct {
si SchemaInformation

Tables []*TableInfo
scopes []*scope

exprDeps map[sqlparser.Expr]TableSet
err error
Tables []*TableInfo
scopes []*scope
exprDeps map[sqlparser.Expr]TableSet
err error
currentDb string
}
)

// newAnalyzer create the semantic analyzer
func newAnalyzer(si SchemaInformation) *analyzer {
func newAnalyzer(dbName string, si SchemaInformation) *analyzer {
return &analyzer{
exprDeps: map[sqlparser.Expr]TableSet{},
si: si,
exprDeps: map[sqlparser.Expr]TableSet{},
currentDb: dbName,
si: si,
}
}

// Analyze analyzes the parsed query.
func Analyze(statement sqlparser.Statement, currentDb string, si SchemaInformation) (*SemTable, error) {
analyzer := newAnalyzer(currentDb, si)
// Initial scope
err := analyzer.analyze(statement)
if err != nil {
return nil, err
}
return &SemTable{exprDependencies: analyzer.exprDeps, Tables: analyzer.Tables}, nil
}

// analyzeDown pushes new scopes when we encounter sub queries,
Expand Down Expand Up @@ -71,10 +83,11 @@ func (a *analyzer) analyzeDown(cursor *sqlparser.Cursor) bool {
t, err := a.resolveColumn(node, current)
if err != nil {
a.err = err
} else {
a.exprDeps[node] = t
}
a.exprDeps[node] = t
}
return a.shouldContinue()
return true // we always return true here, and then return false on the going up phase to abort on error
harshit-gangal marked this conversation as resolved.
Show resolved Hide resolved
}

func (a *analyzer) resolveColumn(colName *sqlparser.ColName, current *scope) (TableSet, error) {
Expand Down Expand Up @@ -103,9 +116,6 @@ func (a *analyzer) analyzeTableExprs(tablExprs sqlparser.TableExprs) error {
func (a *analyzer) analyzeTableExpr(tableExpr sqlparser.TableExpr) error {
switch table := tableExpr.(type) {
case *sqlparser.AliasedTableExpr:
if !table.As.IsEmpty() {
return Gen4NotSupportedF("table aliases")
}
return a.bindTable(table, table.Expr)
case *sqlparser.JoinTableExpr:
if table.Join != sqlparser.NormalJoinType {
Expand All @@ -125,16 +135,18 @@ func (a *analyzer) analyzeTableExpr(tableExpr sqlparser.TableExpr) error {

// resolveQualifiedColumn handles `tabl.col` expressions
func (a *analyzer) resolveQualifiedColumn(current *scope, expr *sqlparser.ColName) (*TableInfo, error) {
qualifier := expr.Qualifier.Name.String()

// search up the scope stack until we find a match
for current != nil {
tableExpr, found := current.tables[qualifier]
if found {
return tableExpr, nil
dbName := expr.Qualifier.Qualifier.String()
tableName := expr.Qualifier.Name.String()
for _, table := range current.tables {
if tableName == table.tableName &&
(dbName == table.dbName || (dbName == "" && (table.dbName == a.currentDb || a.currentDb == ""))) {
return table, nil
}
}
current = current.parent
}

return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.BadFieldError, "Unknown table referenced by '%s'", sqlparser.String(expr))
}

Expand Down Expand Up @@ -181,7 +193,8 @@ func (a *analyzer) bindTable(alias *sqlparser.AliasedTableExpr, expr sqlparser.S
}
a.popScope()
scope := a.currentScope()
return scope.addTable(alias.As.String(), &TableInfo{alias, nil})
//dbName := "" // derived tables are always referenced only by their alias - they cannot be found using a fully qualified name
return scope.addTable(&TableInfo{})
case sqlparser.TableName:
tbl, vdx, _, _, _, err := a.si.FindTableOrVindex(t)
if err != nil {
Expand All @@ -191,12 +204,24 @@ func (a *analyzer) bindTable(alias *sqlparser.AliasedTableExpr, expr sqlparser.S
return Gen4NotSupportedF("vindex in FROM")
}
scope := a.currentScope()
table := &TableInfo{alias, tbl}
a.Tables = append(a.Tables, table)
dbName := t.Qualifier.String()
if dbName == "" {
dbName = a.currentDb
}
var tableName string
if alias.As.IsEmpty() {
return scope.addTable(t.Name.String(), table)
tableName = t.Name.String()
} else {
tableName = alias.As.String()
}
table := &TableInfo{
dbName: dbName,
tableName: tableName,
ASTNode: alias,
Table: tbl,
}
return scope.addTable(alias.As.String(), table)
a.Tables = append(a.Tables, table)
return scope.addTable(table)
}
return nil
}
Expand All @@ -211,7 +236,7 @@ func (a *analyzer) analyzeUp(cursor *sqlparser.Cursor) bool {
case *sqlparser.Union, *sqlparser.Select:
a.popScope()
}
return true
return a.shouldContinue()
}

func (a *analyzer) shouldContinue() bool {
Expand Down
Loading