diff --git a/go/vt/sqlparser/ast.go b/go/vt/sqlparser/ast.go index db3d010d4a0..35160dbd3c2 100644 --- a/go/vt/sqlparser/ast.go +++ b/go/vt/sqlparser/ast.go @@ -201,9 +201,9 @@ type ( Distinct bool StraightJoinHint bool SQLCalcFoundRows bool + From []TableExpr Comments Comments SelectExprs SelectExprs - From TableExprs Where *Where GroupBy GroupBy Having *Where diff --git a/go/vt/sqlparser/ast_clone.go b/go/vt/sqlparser/ast_clone.go index 16f9e2ed2c3..901029c6aad 100644 --- a/go/vt/sqlparser/ast_clone.go +++ b/go/vt/sqlparser/ast_clone.go @@ -1342,9 +1342,9 @@ func CloneRefOfSelect(n *Select) *Select { } out := *n out.Cache = CloneRefOfBool(n.Cache) + out.From = CloneSliceOfTableExpr(n.From) out.Comments = CloneComments(n.Comments) out.SelectExprs = CloneSelectExprs(n.SelectExprs) - out.From = CloneTableExprs(n.From) out.Where = CloneRefOfWhere(n.Where) out.GroupBy = CloneGroupBy(n.GroupBy) out.Having = CloneRefOfWhere(n.Having) @@ -2366,6 +2366,15 @@ func CloneRefOfBool(n *bool) *bool { return &out } +// CloneSliceOfTableExpr creates a deep clone of the input. +func CloneSliceOfTableExpr(n []TableExpr) []TableExpr { + res := make([]TableExpr, 0, len(n)) + for _, x := range n { + res = append(res, CloneTableExpr(x)) + } + return res +} + // CloneSliceOfCharacteristic creates a deep clone of the input. func CloneSliceOfCharacteristic(n []Characteristic) []Characteristic { res := make([]Characteristic, 0, len(n)) diff --git a/go/vt/sqlparser/ast_equals.go b/go/vt/sqlparser/ast_equals.go index 1107a2489cc..7f3ccf22cba 100644 --- a/go/vt/sqlparser/ast_equals.go +++ b/go/vt/sqlparser/ast_equals.go @@ -2149,9 +2149,9 @@ func EqualsRefOfSelect(a, b *Select) bool { a.StraightJoinHint == b.StraightJoinHint && a.SQLCalcFoundRows == b.SQLCalcFoundRows && EqualsRefOfBool(a.Cache, b.Cache) && + EqualsSliceOfTableExpr(a.From, b.From) && EqualsComments(a.Comments, b.Comments) && EqualsSelectExprs(a.SelectExprs, b.SelectExprs) && - EqualsTableExprs(a.From, b.From) && EqualsRefOfWhere(a.Where, b.Where) && EqualsGroupBy(a.GroupBy, b.GroupBy) && EqualsRefOfWhere(a.Having, b.Having) && @@ -3890,6 +3890,19 @@ func EqualsRefOfBool(a, b *bool) bool { return *a == *b } +// EqualsSliceOfTableExpr does deep equals between the two objects. +func EqualsSliceOfTableExpr(a, b []TableExpr) bool { + if len(a) != len(b) { + return false + } + for i := 0; i < len(a); i++ { + if !EqualsTableExpr(a[i], b[i]) { + return false + } + } + return true +} + // EqualsSliceOfCharacteristic does deep equals between the two objects. func EqualsSliceOfCharacteristic(a, b []Characteristic) bool { if len(a) != len(b) { diff --git a/go/vt/sqlparser/ast_format.go b/go/vt/sqlparser/ast_format.go index 1ce0e24c301..43e1f7f716f 100644 --- a/go/vt/sqlparser/ast_format.go +++ b/go/vt/sqlparser/ast_format.go @@ -43,9 +43,16 @@ func (node *Select) Format(buf *TrackedBuffer) { buf.WriteString(SQLCalcFoundRowsStr) } - buf.astPrintf(node, "%v from %v%v%v%v%v%v%s%v", - node.SelectExprs, - node.From, node.Where, + buf.astPrintf(node, "%v from ", node.SelectExprs) + + prefix := "" + for _, expr := range node.From { + buf.astPrintf(node, "%s%v", prefix, expr) + prefix = ", " + } + + buf.astPrintf(node, "%v%v%v%v%v%s%v", + node.Where, node.GroupBy, node.Having, node.OrderBy, node.Limit, node.Lock.ToString(), node.Into) } diff --git a/go/vt/sqlparser/ast_format_fast.go b/go/vt/sqlparser/ast_format_fast.go index 4302c556de2..4dd76509ab7 100644 --- a/go/vt/sqlparser/ast_format_fast.go +++ b/go/vt/sqlparser/ast_format_fast.go @@ -48,7 +48,12 @@ func (node *Select) formatFast(buf *TrackedBuffer) { node.SelectExprs.formatFast(buf) buf.WriteString(" from ") - node.From.formatFast(buf) + prefix := "" + for _, expr := range node.From { + buf.WriteString(prefix) + expr.formatFast(buf) + prefix = ", " + } node.Where.formatFast(buf) diff --git a/go/vt/sqlparser/ast_funcs.go b/go/vt/sqlparser/ast_funcs.go index 84b32180937..5029e6d5691 100644 --- a/go/vt/sqlparser/ast_funcs.go +++ b/go/vt/sqlparser/ast_funcs.go @@ -1339,3 +1339,15 @@ func handleUnaryMinus(expr Expr) Expr { func encodeSQLString(val string) string { return sqltypes.EncodeStringSQL(val) } + +// ToString prints the list of table expressions as a string +// To be used as an alternate for String for []TableExpr +func ToString(exprs []TableExpr) string { + buf := NewTrackedBuffer(nil) + prefix := "" + for _, expr := range exprs { + buf.astPrintf(nil, "%s%v", prefix, expr) + prefix = ", " + } + return buf.String() +} diff --git a/go/vt/sqlparser/ast_rewrite.go b/go/vt/sqlparser/ast_rewrite.go index 0ca9fbf25a6..c54f1feff36 100644 --- a/go/vt/sqlparser/ast_rewrite.go +++ b/go/vt/sqlparser/ast_rewrite.go @@ -3320,6 +3320,15 @@ func (a *application) rewriteRefOfSelect(parent SQLNode, node *Select, replacer return true } } + for x, el := range node.From { + if !a.rewriteTableExpr(node, el, func(idx int) replacerFunc { + return func(newNode, parent SQLNode) { + parent.(*Select).From[idx] = newNode.(TableExpr) + } + }(x)) { + return false + } + } if !a.rewriteComments(node, node.Comments, func(newNode, parent SQLNode) { parent.(*Select).Comments = newNode.(Comments) }) { @@ -3330,11 +3339,6 @@ func (a *application) rewriteRefOfSelect(parent SQLNode, node *Select, replacer }) { return false } - if !a.rewriteTableExprs(node, node.From, func(newNode, parent SQLNode) { - parent.(*Select).From = newNode.(TableExprs) - }) { - return false - } if !a.rewriteRefOfWhere(node, node.Where, func(newNode, parent SQLNode) { parent.(*Select).Where = newNode.(*Where) }) { diff --git a/go/vt/sqlparser/ast_visit.go b/go/vt/sqlparser/ast_visit.go index d2d1aafa99f..9acffb6323c 100644 --- a/go/vt/sqlparser/ast_visit.go +++ b/go/vt/sqlparser/ast_visit.go @@ -1679,15 +1679,17 @@ func VisitRefOfSelect(in *Select, f Visit) error { if cont, err := f(in); err != nil || !cont { return err } + for _, el := range in.From { + if err := VisitTableExpr(el, f); err != nil { + return err + } + } if err := VisitComments(in.Comments, f); err != nil { return err } if err := VisitSelectExprs(in.SelectExprs, f); err != nil { return err } - if err := VisitTableExprs(in.From, f); err != nil { - return err - } if err := VisitRefOfWhere(in.Where, f); err != nil { return err } diff --git a/go/vt/sqlparser/cached_size.go b/go/vt/sqlparser/cached_size.go index c34d52e057b..1aa0a172e9e 100644 --- a/go/vt/sqlparser/cached_size.go +++ b/go/vt/sqlparser/cached_size.go @@ -1676,6 +1676,15 @@ func (cached *Select) CachedSize(alloc bool) int64 { } // field Cache *bool size += int64(1) + // field From []vitess.io/vitess/go/vt/sqlparser.TableExpr + { + size += int64(cap(cached.From)) * int64(16) + for _, elem := range cached.From { + if cc, ok := elem.(cachedObject); ok { + size += cc.CachedSize(true) + } + } + } // field Comments vitess.io/vitess/go/vt/sqlparser.Comments { size += int64(cap(cached.Comments)) * int64(16) @@ -1692,15 +1701,6 @@ func (cached *Select) CachedSize(alloc bool) int64 { } } } - // field From vitess.io/vitess/go/vt/sqlparser.TableExprs - { - size += int64(cap(cached.From)) * int64(16) - for _, elem := range cached.From { - if cc, ok := elem.(cachedObject); ok { - size += cc.CachedSize(true) - } - } - } // field Where *vitess.io/vitess/go/vt/sqlparser.Where size += cached.Where.CachedSize(true) // field GroupBy vitess.io/vitess/go/vt/sqlparser.GroupBy diff --git a/go/vt/sqlparser/impossible_query.go b/go/vt/sqlparser/impossible_query.go index 2437b551587..dc8a2a931a1 100644 --- a/go/vt/sqlparser/impossible_query.go +++ b/go/vt/sqlparser/impossible_query.go @@ -27,7 +27,13 @@ package sqlparser func FormatImpossibleQuery(buf *TrackedBuffer, node SQLNode) { switch node := node.(type) { case *Select: - buf.Myprintf("select %v from %v where 1 != 1", node.SelectExprs, node.From) + buf.Myprintf("select %v from ", node.SelectExprs) + var prefix string + for _, n := range node.From { + buf.Myprintf("%s%v", prefix, n) + prefix = ", " + } + buf.Myprintf(" where 1 != 1") if node.GroupBy != nil { node.GroupBy.Format(buf) } diff --git a/go/vt/vtctl/workflow/vexec/vexec.go b/go/vt/vtctl/workflow/vexec/vexec.go index 93c5359d068..5f9f2841718 100644 --- a/go/vt/vtctl/workflow/vexec/vexec.go +++ b/go/vt/vtctl/workflow/vexec/vexec.go @@ -265,7 +265,7 @@ func extractTableName(stmt sqlparser.Statement) (string, error) { case *sqlparser.Insert: return sqlparser.String(stmt.Table), nil case *sqlparser.Select: - return sqlparser.String(stmt.From), nil + return sqlparser.ToString(stmt.From), nil } return "", fmt.Errorf("%w: %+v", ErrUnsupportedQuery, sqlparser.String(stmt)) diff --git a/go/vt/vtgate/planbuilder/joinGen4.go b/go/vt/vtgate/planbuilder/joinGen4.go index 67f26ff341d..4339f1f2452 100644 --- a/go/vt/vtgate/planbuilder/joinGen4.go +++ b/go/vt/vtgate/planbuilder/joinGen4.go @@ -17,7 +17,9 @@ limitations under the License. package planbuilder import ( + vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" "vitess.io/vitess/go/vt/vtgate/semantics" ) @@ -91,12 +93,17 @@ func (j *joinGen4) Primitive() engine.Primitive { // Inputs implements the logicalPlan interface func (j *joinGen4) Inputs() []logicalPlan { - panic("implement me") + return []logicalPlan{j.Left, j.Right} } // Rewrite implements the logicalPlan interface func (j *joinGen4) Rewrite(inputs ...logicalPlan) error { - panic("implement me") + if len(inputs) != 2 { + return vterrors.New(vtrpcpb.Code_INTERNAL, "wrong number of children") + } + j.Left = inputs[0] + j.Right = inputs[1] + return nil } // ContainsTables implements the logicalPlan interface diff --git a/go/vt/vtgate/planbuilder/route_planning.go b/go/vt/vtgate/planbuilder/route_planning.go index a955caf09d7..be4e7feebe9 100644 --- a/go/vt/vtgate/planbuilder/route_planning.go +++ b/go/vt/vtgate/planbuilder/route_planning.go @@ -261,13 +261,10 @@ func planHorizon(sel *sqlparser.Select, plan logicalPlan, semTable *semantics.Se return plan, nil } - // TODO real horizon planning to be done - if sel.Distinct { - return nil, semantics.Gen4NotSupportedF("DISTINCT") - } - if sel.GroupBy != nil { - return nil, semantics.Gen4NotSupportedF("GROUP BY") + if err := checkUnsupportedConstructs(sel); err != nil { + return nil, err } + qp, err := createQPFromSelect(sel) if err != nil { return nil, err @@ -278,6 +275,16 @@ func planHorizon(sel *sqlparser.Select, plan logicalPlan, semTable *semantics.Se } } + for _, expr := range qp.aggrExprs { + funcExpr, ok := expr.Expr.(*sqlparser.FuncExpr) + if !ok { + return nil, vterrors.New(vtrpcpb.Code_INTERNAL, "expected an aggregation here") + } + if funcExpr.Distinct { + return nil, semantics.Gen4NotSupportedF("distinct aggregation") + } + } + if len(qp.aggrExprs) > 0 { plan, err = planAggregations(qp, plan, semTable) if err != nil { @@ -308,6 +315,19 @@ func createSingleShardRoutePlan(sel *sqlparser.Select, rb *route) { } } +func checkUnsupportedConstructs(sel *sqlparser.Select) error { + if sel.Distinct { + return semantics.Gen4NotSupportedF("DISTINCT") + } + if sel.GroupBy != nil { + return semantics.Gen4NotSupportedF("GROUP BY") + } + if sel.Having != nil { + return semantics.Gen4NotSupportedF("HAVING") + } + return nil +} + func pushJoinPredicate(exprs []sqlparser.Expr, tree joinTree, semTable *semantics.SemTable) (joinTree, error) { switch node := tree.(type) { case *routePlan: diff --git a/go/vt/vtgate/planbuilder/symtab.go b/go/vt/vtgate/planbuilder/symtab.go index afaffea1f97..658f52d5c9e 100644 --- a/go/vt/vtgate/planbuilder/symtab.go +++ b/go/vt/vtgate/planbuilder/symtab.go @@ -172,7 +172,7 @@ func (st *symtab) AddTable(t *table) error { st.singleRoute = nil } if _, ok := st.tables[t.alias]; ok { - return fmt.Errorf("duplicate symbol: %s", sqlparser.String(t.alias)) + return vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.NonUniqTable, "Not unique table/alias: '%s'", t.alias.Name.String()) } st.tables[t.alias] = t st.tableNames = append(st.tableNames, t.alias) diff --git a/go/vt/vtgate/planbuilder/testdata/aggr_cases.txt b/go/vt/vtgate/planbuilder/testdata/aggr_cases.txt index e4a697bb9c8..b109137932f 100644 --- a/go/vt/vtgate/planbuilder/testdata/aggr_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/aggr_cases.txt @@ -1342,3 +1342,4 @@ Gen4 plan same as above # syntax error detected by planbuilder "select count(distinct *) from user" "syntax error: count(distinct *)" +Gen4 plan same as above diff --git a/go/vt/vtgate/planbuilder/testdata/ddl_cases_no_default_keyspace.txt b/go/vt/vtgate/planbuilder/testdata/ddl_cases_no_default_keyspace.txt index 190ed35ad1b..50afcc55b80 100644 --- a/go/vt/vtgate/planbuilder/testdata/ddl_cases_no_default_keyspace.txt +++ b/go/vt/vtgate/planbuilder/testdata/ddl_cases_no_default_keyspace.txt @@ -42,6 +42,7 @@ "Query": "create view view_a as select 1 from `user`" } } +Gen4 plan same as above # create view with '*' expression for simple route "create view user.view_a as select user.* from user" @@ -102,6 +103,18 @@ "Query": "create view view_a as select * from authoritative" } } +{ + "QueryType": "DDL", + "Original": "create view user.view_a as select * from authoritative", + "Instructions": { + "OperatorType": "DDL", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "Query": "create view view_a as select authoritative.user_id as user_id, authoritative.col1 as col1, authoritative.col2 as col2 from authoritative" + } +} # create view with select * from join of authoritative tables "create view user.view_a as select * from authoritative a join authoritative b on a.user_id=b.user_id" @@ -117,6 +130,18 @@ "Query": "create view view_a as select * from authoritative as a join authoritative as b on a.user_id = b.user_id" } } +{ + "QueryType": "DDL", + "Original": "create view user.view_a as select * from authoritative a join authoritative b on a.user_id=b.user_id", + "Instructions": { + "OperatorType": "DDL", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "Query": "create view view_a as select a.user_id as user_id, a.col1 as col1, a.col2 as col2, b.user_id as user_id, b.col1 as col1, b.col2 as col2 from authoritative as a join authoritative as b on a.user_id = b.user_id" + } +} # create view with select * from qualified authoritative table "create view user.view_a as select a.* from authoritative a" @@ -132,6 +157,18 @@ "Query": "create view view_a as select a.* from authoritative as a" } } +{ + "QueryType": "DDL", + "Original": "create view user.view_a as select a.* from authoritative a", + "Instructions": { + "OperatorType": "DDL", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "Query": "create view view_a as select a.user_id as user_id, a.col1 as col1, a.col2 as col2 from authoritative as a" + } +} # create view with select * from intermixing of authoritative table with non-authoritative results in no expansion "create view user.view_a as select * from authoritative join user on authoritative.user_id=user.id" @@ -147,7 +184,6 @@ "Query": "create view view_a as select * from authoritative join `user` on authoritative.user_id = `user`.id" } } - # create view with select authoritative.* with intermixing still expands "create view user.view_a as select user.id, a.*, user.col1 from authoritative a join user on a.user_id=user.id" { @@ -162,6 +198,18 @@ "Query": "create view view_a as select `user`.id, a.*, `user`.col1 from authoritative as a join `user` on a.user_id = `user`.id" } } +{ + "QueryType": "DDL", + "Original": "create view user.view_a as select user.id, a.*, user.col1 from authoritative a join user on a.user_id=user.id", + "Instructions": { + "OperatorType": "DDL", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "Query": "create view view_a as select `user`.id, a.user_id as user_id, a.col1 as col1, a.col2 as col2, `user`.col1 from authoritative as a join `user` on a.user_id = `user`.id" + } +} # create view with auto-resolve anonymous columns for simple route "create view user.view_a as select col from user join user_extra on user.id = user_extra.user_id" @@ -192,6 +240,7 @@ "Query": "create view view_a as select `user`.id from `user` join user_extra on `user`.id = user_extra.user_id" } } +Gen4 plan same as above # create view with last_insert_id for unsharded route "create view main.view_a as select last_insert_id() as x from main.unsharded" @@ -207,6 +256,7 @@ "Query": "create view view_a as select :__lastInsertId as x from unsharded" } } +Gen4 plan same as above # create view with select from pinned table "create view user.view_a as select * from pin_test" @@ -222,6 +272,7 @@ "Query": "create view view_a as select * from pin_test" } } +Gen4 plan same as above # create view with Expression with single-route reference "create view user.view_a as select user.col, user_extra.id + user_extra.col from user join user_extra on user.id = user_extra.user_id" @@ -237,6 +288,7 @@ "Query": "create view view_a as select `user`.col, user_extra.id + user_extra.col from `user` join user_extra on `user`.id = user_extra.user_id" } } +Gen4 plan same as above # create view with Comments "create view user.view_a as select /* comment */ user.col from user join user_extra on user.id = user_extra.user_id" @@ -252,6 +304,7 @@ "Query": "create view view_a as select /* comment */ `user`.col from `user` join user_extra on `user`.id = user_extra.user_id" } } +Gen4 plan same as above # create view with for update "create view user.view_a as select user.col from user join user_extra on user.id = user_extra.user_id for update" @@ -267,6 +320,7 @@ "Query": "create view view_a as select `user`.col from `user` join user_extra on `user`.id = user_extra.user_id for update" } } +Gen4 plan same as above # create view with Case preservation "create view user.view_a as select user.Col, user_extra.Id from user join user_extra on user.id = user_extra.user_id" @@ -282,10 +336,12 @@ "Query": "create view view_a as select `user`.Col, user_extra.Id from `user` join user_extra on `user`.id = user_extra.user_id" } } +Gen4 plan same as above # create view with syntax error "create view user.view_a as the quick brown fox" "syntax error at position 31 near 'the'" +Gen4 plan same as above # create view with Hex number is not treated as a simple value "create view user.view_a as select * from user where id = 0x04" @@ -316,6 +372,7 @@ "Query": "create view view_a as select * from `user` where `name` = 'abc' and id = 4 limit 5" } } +Gen4 plan same as above # create view with Multiple parenthesized expressions "create view user.view_a as select * from user where (id = 4) AND (name ='abc') limit 5" @@ -331,6 +388,7 @@ "Query": "create view view_a as select * from `user` where id = 4 and `name` = 'abc' limit 5" } } +Gen4 plan same as above # create view with Multiple parenthesized expressions "create view user.view_a as select * from user where (id = 4 and name ='abc') limit 5" @@ -346,6 +404,7 @@ "Query": "create view view_a as select * from `user` where id = 4 and `name` = 'abc' limit 5" } } +Gen4 plan same as above # create view with Column Aliasing with Table.Column "create view user.view_a as select user0_.col as col0_ from user user0_ where id = 1 order by user0_.col" @@ -361,6 +420,7 @@ "Query": "create view view_a as select user0_.col as col0_ from `user` as user0_ where id = 1 order by user0_.col asc" } } +Gen4 plan same as above # create view with Column Aliasing with Column "create view user.view_a as select user0_.col as col0_ from user user0_ where id = 1 order by col0_ desc" @@ -376,6 +436,7 @@ "Query": "create view view_a as select user0_.col as col0_ from `user` as user0_ where id = 1 order by col0_ desc" } } +Gen4 plan same as above # create view with Booleans and parenthesis "create view user.view_a as select * from user where (id = 1) AND name = true" @@ -391,6 +452,7 @@ "Query": "create view view_a as select * from `user` where id = 1 and `name` = true" } } +Gen4 plan same as above # create view with union with the same target shard "create view user.view_a as select * from music where user_id = 1 union select * from user where id = 1" @@ -406,6 +468,7 @@ "Query": "create view view_a as select * from music where user_id = 1 union select * from `user` where id = 1" } } +Gen4 plan same as above # create view with testing SingleRow Projection "create view user.view_a as select 42 from user" @@ -421,6 +484,7 @@ "Query": "create view view_a as select 42 from `user`" } } +Gen4 plan same as above # create view with sql_calc_found_rows without limit "create view user.view_a as select sql_calc_found_rows * from music where user_id = 1" @@ -436,6 +500,18 @@ "Query": "create view view_a as select * from music where user_id = 1" } } +{ + "QueryType": "DDL", + "Original": "create view user.view_a as select sql_calc_found_rows * from music where user_id = 1", + "Instructions": { + "OperatorType": "DDL", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "Query": "create view view_a as select sql_calc_found_rows * from music where user_id = 1" + } +} # DDL "create index a on user(id)" @@ -451,6 +527,7 @@ "Query": "alter table `user` add index a (id)" } } +Gen4 plan same as above #Alter table with qualifier "alter table user ADD id int" @@ -466,6 +543,7 @@ "Query": "alter table `user` add column id int" } } +Gen4 plan same as above # Alter View "alter view user_extra as select* from user" @@ -485,6 +563,7 @@ # Alter View with unknown view "alter view unknown as select* from user" "keyspace not specified" +Gen4 plan same as above # drop table with qualifier in one "drop table user.user, user_extra" @@ -500,14 +579,17 @@ "Query": "drop table `user`, user_extra" } } +Gen4 plan same as above # drop table with incompatible tables "drop table user, unsharded_a" "Tables or Views specified in the query do not belong to the same destination" +Gen4 plan same as above # drop table with unknown table "drop table unknown" "keyspace not specified" +Gen4 plan same as above # drop view with 1 view without qualifier "drop view user.user, user_extra" @@ -523,14 +605,17 @@ "Query": "drop view `user`, user_extra" } } +Gen4 plan same as above # drop view with incompatible views "drop view user, unsharded_a" "Tables or Views specified in the query do not belong to the same destination" +Gen4 plan same as above # drop view with unknown view "drop view unknown" "keyspace not specified" +Gen4 plan same as above # Truncate table without qualifier "truncate user_extra" @@ -546,6 +631,7 @@ "Query": "truncate table user_extra" } } +Gen4 plan same as above # Rename table "rename table user_extra to b" @@ -561,11 +647,14 @@ "Query": "rename table user_extra to b" } } +Gen4 plan same as above # Rename table with different keyspace tables "rename table user_extra to b, main.a to b" "Tables or Views specified in the query do not belong to the same destination" +Gen4 plan same as above # Rename table with change in keyspace name "rename table user_extra to main.b" "Changing schema from 'user' to 'main' is not allowed" +Gen4 plan same as above diff --git a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt index c8370b75622..4be2688c686 100644 --- a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt @@ -1280,6 +1280,7 @@ Gen4 plan same as above # unresolved symbol in inner subquery. "select id from user where id = :a and user.col in (select user_extra.col from user_extra where user_extra.user_id = :a and foo.id = 1)" "symbol foo.id not found" +Gen4 plan same as above # outer and inner subquery route by same outermost column value "select id2 from user uu where id in (select id from user where id = uu.id and user.col in (select user_extra.col from user_extra where user_extra.user_id = uu.id))" @@ -1999,3 +2000,44 @@ Gen4 plan same as above } } Gen4 plan same as above + + +"select * from samecolvin where col = :col" +{ + "QueryType": "SELECT", + "Original": "select * from samecolvin where col = :col", + "Instructions": { + "OperatorType": "Route", + "Variant": "SelectEqualUnique", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select col from samecolvin where 1 != 1", + "Query": "select col from samecolvin where col = :col", + "Table": "samecolvin", + "Values": [ + ":col" + ], + "Vindex": "vindex1" + } +} +{ + "QueryType": "SELECT", + "Original": "select * from samecolvin where col = :col", + "Instructions": { + "OperatorType": "Route", + "Variant": "SelectEqualUnique", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select samecolvin.col as col from samecolvin where 1 != 1", + "Query": "select samecolvin.col as col from samecolvin where col = :col", + "Table": "samecolvin", + "Values": [ + ":col" + ], + "Vindex": "vindex1" + } +} diff --git a/go/vt/vtgate/planbuilder/testdata/flush_cases_no_default_keyspace.txt b/go/vt/vtgate/planbuilder/testdata/flush_cases_no_default_keyspace.txt index 19e25ca060b..e0e4d3a10f4 100644 --- a/go/vt/vtgate/planbuilder/testdata/flush_cases_no_default_keyspace.txt +++ b/go/vt/vtgate/planbuilder/testdata/flush_cases_no_default_keyspace.txt @@ -27,10 +27,12 @@ ] } } +Gen4 plan same as above # Flush statement with flush options "flush no_write_to_binlog hosts, logs" "keyspace not specified" +Gen4 plan same as above # Flush statement with routing rules "flush local tables route1, route2" @@ -61,10 +63,12 @@ ] } } +Gen4 plan same as above # Incorrect tables in flush "flush tables user.a with read lock" "table a not found" +Gen4 plan same as above # Unknown tables in unsharded keyspaces are allowed "flush tables main.a with read lock" @@ -81,6 +85,7 @@ "Query": "flush tables a with read lock" } } +Gen4 plan same as above # Flush statement with 3 keyspaces "flush local tables user, unsharded_a, user_extra, unsharded_tab with read lock" @@ -120,3 +125,4 @@ ] } } +Gen4 plan same as above diff --git a/go/vt/vtgate/planbuilder/testdata/from_cases.txt b/go/vt/vtgate/planbuilder/testdata/from_cases.txt index a90000dcea8..3a0e1b308de 100644 --- a/go/vt/vtgate/planbuilder/testdata/from_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/from_cases.txt @@ -2685,6 +2685,7 @@ Gen4 plan same as above # verify ',' vs JOIN precedence "select u1.a from unsharded u1, unsharded u2 join unsharded u3 on u1.a = u2.a" "symbol u1.a not found" +Gen4 plan same as above # first expression fails for ',' join (code coverage: ensure error is returned) "select user.foo.col from user.foo, user" @@ -2694,6 +2695,7 @@ Gen4 plan same as above # table names should be case-sensitive "select unsharded.id from unsharded where Unsharded.val = 1" "symbol Unsharded.val not found" +Gen4 plan same as above # implicit table reference for sharded keyspace "select user.foo.col from user.foo" @@ -2702,11 +2704,13 @@ Gen4 plan same as above # duplicate symbols "select user.id from user join user" -"duplicate symbol: `user`" +"Not unique table/alias: 'user'" +Gen4 plan same as above # duplicate symbols for merging routes "select user.id from user join user_extra user on user.id = user.user_id" -"duplicate symbol: `user`" +"Not unique table/alias: 'user'" +Gen4 plan same as above # non-existent table "select c from t" diff --git a/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt b/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt index 55fe2c9ff42..e204b6fea3d 100644 --- a/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt @@ -822,6 +822,7 @@ Gen4 plan same as above # Order by, qualified '*' expression, name mismatched. "select user.* from user where id = 5 order by e.col" "symbol e.col not found" +Gen4 plan same as above # Order by, invalid column number "select col from user order by 18446744073709551616" diff --git a/go/vt/vtgate/planbuilder/testdata/vindex_func_cases.txt b/go/vt/vtgate/planbuilder/testdata/vindex_func_cases.txt index d2ae819cd37..c81faa8c7cb 100644 --- a/go/vt/vtgate/planbuilder/testdata/vindex_func_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/vindex_func_cases.txt @@ -303,24 +303,3 @@ Gen4 plan same as above "select none from user_index where id = :id" "symbol `none` not found in table or subquery" - -"select * from samecolvin where col = :col" -{ - "QueryType": "SELECT", - "Original": "select * from samecolvin where col = :col", - "Instructions": { - "OperatorType": "Route", - "Variant": "SelectEqualUnique", - "Keyspace": { - "Name": "user", - "Sharded": true - }, - "FieldQuery": "select col from samecolvin where 1 != 1", - "Query": "select col from samecolvin where col = :col", - "Table": "samecolvin", - "Values": [ - ":col" - ], - "Vindex": "vindex1" - } -} diff --git a/go/vt/vtgate/planbuilder/testdata/wireup_cases.txt b/go/vt/vtgate/planbuilder/testdata/wireup_cases.txt index 190692c0d06..80ca3f3367a 100644 --- a/go/vt/vtgate/planbuilder/testdata/wireup_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/wireup_cases.txt @@ -127,6 +127,59 @@ ] } } +{ + "QueryType": "SELECT", + "Original": "select u1.id from user u1 join user u2 join user u3 where u3.col = u1.col", + "Instructions": { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "1", + "TableName": "`user`_`user`_`user`", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select 1 from `user` as u2 where 1 != 1", + "Query": "select 1 from `user` as u2", + "Table": "`user`" + }, + { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "-2", + "TableName": "`user`_`user`", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select u1.col, u1.id from `user` as u1 where 1 != 1", + "Query": "select u1.col, u1.id from `user` as u1", + "Table": "`user`" + }, + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select 1 from `user` as u3 where 1 != 1", + "Query": "select 1 from `user` as u3 where u3.col = :u1_col", + "Table": "`user`" + } + ] + } + ] + } +} # wire-up join with join, going left, then right "select u1.id from user u1 join user u2 join user u3 where u3.col = u2.col" @@ -183,6 +236,58 @@ ] } } +{ + "QueryType": "SELECT", + "Original": "select u1.id from user u1 join user u2 join user u3 where u3.col = u2.col", + "Instructions": { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "-1", + "TableName": "`user`_`user`_`user`", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select u1.id from `user` as u1 where 1 != 1", + "Query": "select u1.id from `user` as u1", + "Table": "`user`" + }, + { + "OperatorType": "Join", + "Variant": "Join", + "TableName": "`user`_`user`", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select u2.col from `user` as u2 where 1 != 1", + "Query": "select u2.col from `user` as u2", + "Table": "`user`" + }, + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select 1 from `user` as u3 where 1 != 1", + "Query": "select 1 from `user` as u3 where u3.col = :u2_col", + "Table": "`user`" + } + ] + } + ] + } +} # wire-up join with join, reuse existing result from a lower join "select u1.id from user u1 join user u2 on u2.col = u1.col join user u3 where u3.col = u1.col" @@ -239,6 +344,59 @@ ] } } +{ + "QueryType": "SELECT", + "Original": "select u1.id from user u1 join user u2 on u2.col = u1.col join user u3 where u3.col = u1.col", + "Instructions": { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "1", + "TableName": "`user`_`user`_`user`", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select u3.col from `user` as u3 where 1 != 1", + "Query": "select u3.col from `user` as u3", + "Table": "`user`" + }, + { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "-2", + "TableName": "`user`_`user`", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select u1.col, u1.id from `user` as u1 where 1 != 1", + "Query": "select u1.col, u1.id from `user` as u1", + "Table": "`user`" + }, + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select 1 from `user` as u2 where 1 != 1", + "Query": "select 1 from `user` as u2 where u2.col = :u1_col and :u3_col = :u1_col", + "Table": "`user`" + } + ] + } + ] + } +} # wire-up join with join, reuse existing result from a lower join. # You need two levels of join nesting to test this: when u3 requests @@ -321,6 +479,82 @@ ] } } +{ + "QueryType": "SELECT", + "Original": "select u1.id from user u1 join user u2 join user u3 on u3.id = u1.col join user u4 where u4.col = u1.col", + "Instructions": { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "1", + "TableName": "`user`_`user`_`user`_`user`", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select 1 from `user` as u2 where 1 != 1", + "Query": "select 1 from `user` as u2", + "Table": "`user`" + }, + { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "1", + "TableName": "`user`_`user`_`user`", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select u4.col from `user` as u4 where 1 != 1", + "Query": "select u4.col from `user` as u4", + "Table": "`user`" + }, + { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "-2", + "TableName": "`user`_`user`", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select u1.col, u1.id from `user` as u1 where 1 != 1", + "Query": "select u1.col, u1.id from `user` as u1", + "Table": "`user`" + }, + { + "OperatorType": "Route", + "Variant": "SelectEqualUnique", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select 1 from `user` as u3 where 1 != 1", + "Query": "select 1 from `user` as u3 where u3.id = :u1_col and :u4_col = :u1_col", + "Table": "`user`", + "Values": [ + ":u1_col" + ], + "Vindex": "user_index" + } + ] + } + ] + } + ] + } +} # Test reuse of join var already being supplied to the right of a node. "select u1.id from user u1 join (user u2 join user u3) where u2.id = u1.col and u3.id = u1.col" @@ -384,6 +618,67 @@ ] } } +{ + "QueryType": "SELECT", + "Original": "select u1.id from user u1 join (user u2 join user u3) where u2.id = u1.col and u3.id = u1.col", + "Instructions": { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "-2", + "TableName": "`user`_`user`_`user`", + "Inputs": [ + { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "-1,-2", + "TableName": "`user`_`user`", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select u1.col, u1.id from `user` as u1 where 1 != 1", + "Query": "select u1.col, u1.id from `user` as u1", + "Table": "`user`" + }, + { + "OperatorType": "Route", + "Variant": "SelectEqualUnique", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select 1 from `user` as u2 where 1 != 1", + "Query": "select 1 from `user` as u2 where u2.id = :u1_col", + "Table": "`user`", + "Values": [ + ":u1_col" + ], + "Vindex": "user_index" + } + ] + }, + { + "OperatorType": "Route", + "Variant": "SelectEqualUnique", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select 1 from `user` as u3 where 1 != 1", + "Query": "select 1 from `user` as u3 where u3.id = :u1_col", + "Table": "`user`", + "Values": [ + ":u1_col" + ], + "Vindex": "user_index" + } + ] + } +} # Join on weird columns. "select `weird``name`.a, unsharded.b from `weird``name` join unsharded on `weird``name`.`a``b*c` = unsharded.id" @@ -421,6 +716,44 @@ ] } } +{ + "QueryType": "SELECT", + "Original": "select `weird``name`.a, unsharded.b from `weird``name` join unsharded on `weird``name`.`a``b*c` = unsharded.id", + "Instructions": { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "1,-2", + "TableName": "unsharded_`weird``name`", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectUnsharded", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "FieldQuery": "select unsharded.id, unsharded.b from unsharded where 1 != 1", + "Query": "select unsharded.id, unsharded.b from unsharded", + "Table": "unsharded" + }, + { + "OperatorType": "Route", + "Variant": "SelectEqualUnique", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select `weird``name`.a from `weird``name` where 1 != 1", + "Query": "select `weird``name`.a from `weird``name` where `weird``name`.`a``b*c` = :unsharded_id", + "Table": "`weird``name`", + "Values": [ + ":unsharded_id" + ], + "Vindex": "user_index" + } + ] + } +} # Join on weird column (col is not in select) "select unsharded.b from `weird``name` join unsharded on `weird``name`.`a``b*c` = unsharded.id" @@ -458,6 +791,44 @@ ] } } +{ + "QueryType": "SELECT", + "Original": "select unsharded.b from `weird``name` join unsharded on `weird``name`.`a``b*c` = unsharded.id", + "Instructions": { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "-2", + "TableName": "unsharded_`weird``name`", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectUnsharded", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "FieldQuery": "select unsharded.id, unsharded.b from unsharded where 1 != 1", + "Query": "select unsharded.id, unsharded.b from unsharded", + "Table": "unsharded" + }, + { + "OperatorType": "Route", + "Variant": "SelectEqualUnique", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select 1 from `weird``name` where 1 != 1", + "Query": "select 1 from `weird``name` where `weird``name`.`a``b*c` = :unsharded_id", + "Table": "`weird``name`", + "Values": [ + ":unsharded_id" + ], + "Vindex": "user_index" + } + ] + } +} # wire-up with limit primitive "select u.id, e.id from user u join user_extra e where e.id = u.col limit 10" @@ -501,6 +872,46 @@ ] } } +{ + "QueryType": "SELECT", + "Original": "select u.id, e.id from user u join user_extra e where e.id = u.col limit 10", + "Instructions": { + "OperatorType": "Limit", + "Count": 10, + "Inputs": [ + { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "-2,1", + "TableName": "`user`_user_extra", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select u.col, u.id from `user` as u where 1 != 1", + "Query": "select u.col, u.id from `user` as u limit :__upper_limit", + "Table": "`user`" + }, + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select e.id from user_extra as e where 1 != 1", + "Query": "select e.id from user_extra as e where e.id = :u_col limit :__upper_limit", + "Table": "user_extra" + } + ] + } + ] + } +} # Wire-up in subquery "select 1 from user where id in (select u.id, e.id from user u join user_extra e where e.id = u.col limit 10)" diff --git a/go/vt/vtgate/semantics/analyzer.go b/go/vt/vtgate/semantics/analyzer.go index 3bb8d2bcd07..8b4993dc1e3 100644 --- a/go/vt/vtgate/semantics/analyzer.go +++ b/go/vt/vtgate/semantics/analyzer.go @@ -67,17 +67,22 @@ func (a *analyzer) analyzeDown(cursor *sqlparser.Cursor) bool { n := cursor.Node() switch node := n.(type) { case *sqlparser.Select: + if node.Having != nil { + a.err = Gen4NotSupportedF("HAVING") + } a.push(newScope(current)) a.selectScope[node] = a.currentScope() - if err := a.analyzeTableExprs(node.From); err != nil { - a.err = err - return false - } case *sqlparser.DerivedTable: a.err = Gen4NotSupportedF("derived tables") - case *sqlparser.TableExprs: - // this has already been visited when we encountered the SELECT struct - return false + case sqlparser.TableExpr: + _, isSelect := cursor.Parent().(*sqlparser.Select) + if isSelect { + a.push(newScope(nil)) + } + switch node := node.(type) { + case *sqlparser.AliasedTableExpr: + a.err = a.bindTable(node, node.Expr) + } // we don't need to push new scope for sub queries since we do that for SELECT and UNION @@ -90,6 +95,18 @@ func (a *analyzer) analyzeDown(cursor *sqlparser.Cursor) bool { } else { a.exprDeps[node] = t } + case *sqlparser.FuncExpr: + if node.Distinct { + err := vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "syntax error: %s", sqlparser.String(node)) + if len(node.Exprs) != 1 { + a.err = err + } else if _, ok := node.Exprs[0].(*sqlparser.AliasedExpr); !ok { + a.err = err + } + } + if sqlparser.IsLockingFunc(node) { + a.err = Gen4NotSupportedF("locking functions") + } } // this is the visitor going down the tree. Returning false here would just not visit the children @@ -112,42 +129,6 @@ func (a *analyzer) resolveColumn(colName *sqlparser.ColName, current *scope) (Ta return a.tableSetFor(t.ASTNode), nil } -func (a *analyzer) analyzeTableExprs(tablExprs sqlparser.TableExprs) error { - for _, tableExpr := range tablExprs { - if err := a.analyzeTableExpr(tableExpr); err != nil { - return err - } - } - return nil -} - -func (a *analyzer) analyzeTableExpr(tableExpr sqlparser.TableExpr) error { - switch table := tableExpr.(type) { - case *sqlparser.AliasedTableExpr: - return a.bindTable(table, table.Expr) - case *sqlparser.JoinTableExpr: - return a.analyzeJoinTableExpr(table) - case *sqlparser.ParenTableExpr: - return a.analyzeTableExprs(table.Exprs) - } - return nil -} - -func (a *analyzer) analyzeJoinTableExpr(table *sqlparser.JoinTableExpr) error { - switch table.Join { - case sqlparser.NormalJoinType, sqlparser.LeftJoinType, sqlparser.RightJoinType: - if err := a.analyzeTableExpr(table.LeftExpr); err != nil { - return err - } - if err := a.analyzeTableExpr(table.RightExpr); err != nil { - return err - } - default: - return Gen4NotSupportedF("join type %s", table.Join.ToString()) - } - return nil -} - // resolveQualifiedColumn handles `tabl.col` expressions func (a *analyzer) resolveQualifiedColumn(current *scope, expr *sqlparser.ColName) (*TableInfo, error) { // search up the scope stack until we find a match @@ -162,7 +143,7 @@ func (a *analyzer) resolveQualifiedColumn(current *scope, expr *sqlparser.ColNam } current = current.parent } - return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.BadFieldError, "Unknown table referenced by '%s'", sqlparser.String(expr)) + return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.BadFieldError, "symbol %s not found", sqlparser.String(expr)) } // resolveUnQualifiedColumn @@ -246,6 +227,21 @@ func (a *analyzer) analyzeUp(cursor *sqlparser.Cursor) bool { switch cursor.Node().(type) { case *sqlparser.Union, *sqlparser.Select: a.popScope() + case sqlparser.TableExpr: + _, isSelect := cursor.Parent().(*sqlparser.Select) + if isSelect { + curScope := a.currentScope() + a.popScope() + earlierScope := a.currentScope() + // copy curScope into the earlierScope + for _, table := range curScope.tables { + err := earlierScope.addTable(table) + if err != nil { + a.err = err + break + } + } + } } return a.shouldContinue() } diff --git a/go/vt/vtgate/semantics/analyzer_test.go b/go/vt/vtgate/semantics/analyzer_test.go index 418f9e273ee..afc2d74d992 100644 --- a/go/vt/vtgate/semantics/analyzer_test.go +++ b/go/vt/vtgate/semantics/analyzer_test.go @@ -213,6 +213,9 @@ func TestBindingMultiTable(t *testing.T) { }, { query: "select b.t.col from b.t, t", deps: T0, + }, { + query: "select u1.a + u2.a from u1, u2", + deps: T0 | T1, }} for _, query := range queries { t.Run(query.query, func(t *testing.T) { @@ -228,6 +231,7 @@ func TestBindingMultiTable(t *testing.T) { "select 1 from d.tabl, d.foo as tabl", "select 1 from d.tabl, d.tabl", "select 1 from d.tabl, tabl", + "select 1 from user join user_extra user", } for _, query := range queries { t.Run(query, func(t *testing.T) { @@ -286,7 +290,7 @@ func TestMissingTable(t *testing.T) { parse, _ := sqlparser.Parse(query) _, err := Analyze(parse, "", &FakeSI{}) require.Error(t, err) - require.Contains(t, err.Error(), "Unknown table") + require.Contains(t, err.Error(), "symbol t.col not found") }) } } @@ -373,6 +377,34 @@ func TestUnknownColumnMap2(t *testing.T) { } } +func TestScoping(t *testing.T) { + queries := []struct { + query string + errorMessage string + }{ + { + query: "select 1 from u1, u2 left join u3 on u1.a = u2.a", + errorMessage: "symbol u1.a not found", + }, + } + for _, query := range queries { + t.Run(query.query, func(t *testing.T) { + parse, err := sqlparser.Parse(query.query) + require.NoError(t, err) + _, err = Analyze(parse, "user", &FakeSI{ + Tables: map[string]*vindexes.Table{ + "t": {Name: sqlparser.NewTableIdent("t")}, + }, + }) + if query.errorMessage == "" { + require.NoError(t, err) + } else { + require.EqualError(t, err, query.errorMessage) + } + }) + } +} + func parseAndAnalyze(t *testing.T, query, dbName string) (sqlparser.Statement, *SemTable) { t.Helper() parse, err := sqlparser.Parse(query) diff --git a/go/vt/vttablet/tabletmanager/vreplication/controller_plan.go b/go/vt/vttablet/tabletmanager/vreplication/controller_plan.go index 2f0da8e9c45..f55c9c715d0 100644 --- a/go/vt/vttablet/tabletmanager/vreplication/controller_plan.go +++ b/go/vt/vttablet/tabletmanager/vreplication/controller_plan.go @@ -221,12 +221,12 @@ func buildDeletePlan(del *sqlparser.Delete) (*controllerPlan, error) { } func buildSelectPlan(sel *sqlparser.Select) (*controllerPlan, error) { - switch sqlparser.String(sel.From) { + switch sqlparser.ToString(sel.From) { case vreplicationTableName, reshardingJournalTableName, copyStateTableName, vreplicationLogTableName: return &controllerPlan{ opcode: selectQuery, }, nil default: - return nil, fmt.Errorf("invalid table name: %v", sqlparser.String(sel.From)) + return nil, fmt.Errorf("invalid table name: %v", sqlparser.ToString(sel.From)) } } diff --git a/go/vt/vttablet/tabletserver/planbuilder/builder.go b/go/vt/vttablet/tabletserver/planbuilder/builder.go index 51b72d3e62f..528825ef3d2 100644 --- a/go/vt/vttablet/tabletserver/planbuilder/builder.go +++ b/go/vt/vttablet/tabletserver/planbuilder/builder.go @@ -45,7 +45,7 @@ func analyzeSelect(sel *sqlparser.Select, tables map[string]*schema.Table) (plan // Check if it's a NEXT VALUE statement. if nextVal, ok := sel.SelectExprs[0].(*sqlparser.Nextval); ok { if plan.Table == nil || plan.Table.Type != schema.Sequence { - return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "%s is not a sequence", sqlparser.String(sel.From)) + return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "%s is not a sequence", sqlparser.ToString(sel.From)) } plan.PlanID = PlanNextval v, err := sqlparser.NewPlanValue(nextVal.Expr) diff --git a/go/vt/vttablet/vexec/vexec.go b/go/vt/vttablet/vexec/vexec.go index 50b84ceb431..dd7b92e4f5b 100644 --- a/go/vt/vttablet/vexec/vexec.go +++ b/go/vt/vttablet/vexec/vexec.go @@ -195,7 +195,7 @@ func (e *TabletVExec) analyzeStatement() error { e.TableName = sqlparser.String(stmt.Table) e.InsertCols = e.analyzeInsertColumns(stmt) case *sqlparser.Select: - e.TableName = sqlparser.String(stmt.From) + e.TableName = sqlparser.ToString(stmt.From) e.WhereCols = e.analyzeWhereEqualsColumns(stmt.Where) default: return fmt.Errorf("query not supported by vexec: %+v", sqlparser.String(stmt)) diff --git a/go/vt/wrangler/vdiff.go b/go/vt/wrangler/vdiff.go index c03d4da471e..61cdc09c133 100644 --- a/go/vt/wrangler/vdiff.go +++ b/go/vt/wrangler/vdiff.go @@ -1108,7 +1108,7 @@ func (td *tableDiffer) genDebugQueryDiff(sel *sqlparser.Select, row []sqltypes.V sel.SelectExprs.Format(buf) } buf.Myprintf(" from ") - sel.From.Format(buf) + buf.Myprintf(sqlparser.ToString(sel.From)) buf.Myprintf(" where ") for i, pkI := range td.selectPks { sel.SelectExprs[pkI].Format(buf) diff --git a/go/vt/wrangler/vexec_plan.go b/go/vt/wrangler/vexec_plan.go index a279a4145e2..8ced8dc9be8 100644 --- a/go/vt/wrangler/vexec_plan.go +++ b/go/vt/wrangler/vexec_plan.go @@ -178,7 +178,7 @@ func extractTableName(stmt sqlparser.Statement) (string, error) { case *sqlparser.Insert: return sqlparser.String(stmt.Table), nil case *sqlparser.Select: - return sqlparser.String(stmt.From), nil + return sqlparser.ToString(stmt.From), nil } return "", fmt.Errorf("query not supported by vexec: %+v", sqlparser.String(stmt)) }