diff --git a/go/vt/vtgate/planbuilder/testdata/from_cases.json b/go/vt/vtgate/planbuilder/testdata/from_cases.json index f47dd91b30b..8315a60e013 100644 --- a/go/vt/vtgate/planbuilder/testdata/from_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/from_cases.json @@ -4588,7 +4588,24 @@ "comment": "verify ',' vs JOIN precedence", "query": "select u1.a from unsharded u1, unsharded u2 join unsharded u3 on u1.a = u2.a", "v3-plan": "VT03019: symbol u1.a not found", - "gen4-plan": "symbol u1.a not found" + "gen4-plan": { + "QueryType": "SELECT", + "Original": "select u1.a from unsharded u1, unsharded u2 join unsharded u3 on u1.a = u2.a", + "Instructions": { + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "FieldQuery": "select u1.a from unsharded as u1, unsharded as u2 join unsharded as u3 on u1.a = u2.a where 1 != 1", + "Query": "select u1.a from unsharded as u1, unsharded as u2 join unsharded as u3 on u1.a = u2.a", + "Table": "unsharded" + }, + "TablesUsed": [ + "main.unsharded" + ] + } }, { "comment": "first expression fails for ',' join (code coverage: ensure error is returned)", @@ -4599,7 +4616,24 @@ "comment": "table names should be case-sensitive", "query": "select unsharded.id from unsharded where Unsharded.val = 1", "v3-plan": "VT03019: symbol Unsharded.val not found", - "gen4-plan": "symbol Unsharded.val not found" + "gen4-plan": { + "QueryType": "SELECT", + "Original": "select unsharded.id from unsharded where Unsharded.val = 1", + "Instructions": { + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "FieldQuery": "select unsharded.id from unsharded where 1 != 1", + "Query": "select unsharded.id from unsharded where Unsharded.val = 1", + "Table": "unsharded" + }, + "TablesUsed": [ + "main.unsharded" + ] + } }, { "comment": "implicit table reference for sharded keyspace", @@ -6343,5 +6377,49 @@ "user.user_extra" ] } + }, + { + "comment": "missing and ambiguous column info is OK as long as we can send the query to a single unsharded keyspace", + "query": "select missing_column from unsharded, unsharded_a", + "v3-plan": { + "QueryType": "SELECT", + "Original": "select missing_column from unsharded, unsharded_a", + "Instructions": { + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "FieldQuery": "select missing_column from unsharded, unsharded_a where 1 != 1", + "Query": "select missing_column from unsharded, unsharded_a", + "Table": "unsharded, unsharded_a" + } + }, + "gen4-plan": { + "QueryType": "SELECT", + "Original": "select missing_column from unsharded, unsharded_a", + "Instructions": { + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "FieldQuery": "select missing_column from unsharded, unsharded_a where 1 != 1", + "Query": "select missing_column from unsharded, unsharded_a", + "Table": "unsharded, unsharded_a" + }, + "TablesUsed": [ + "main.unsharded", + "main.unsharded_a" + ] + } + }, + { + "comment": "missing and ambiguous column info is not valid when we have two different unsharded keyspaces in the query", + "query": "select missing_column from unsharded, unsharded_tab", + "v3-plan": "VT03019: symbol missing_column not found", + "gen4-plan": "Column 'missing_column' in field list is ambiguous" } ] diff --git a/go/vt/vtgate/planbuilder/testdata/union_cases.json b/go/vt/vtgate/planbuilder/testdata/union_cases.json index 5aba78fcb40..cd78c889706 100644 --- a/go/vt/vtgate/planbuilder/testdata/union_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/union_cases.json @@ -2416,5 +2416,42 @@ "Table": "information_schema.key_column_usage" } } + }, + { + "comment": "unknown columns are OK as long as the whole query is unsharded", + "query": "(SELECT * FROM (SELECT * FROM unsharded WHERE branchId = 203622 AND buildNumber <= 113893 AND state = 'FAILED' ORDER BY buildNumber DESC LIMIT 1) AS last_failed) UNION ALL (SELECT * FROM (SELECT * FROM unsharded WHERE branchId = 203622 AND buildNumber <= 113893 AND state = 'SUCCEEDED' ORDER BY buildNumber DESC LIMIT 1) AS last_succeeded) ORDER BY buildNumber DESC LIMIT 1", + "v3-plan": { + "QueryType": "SELECT", + "Original": "(SELECT * FROM (SELECT * FROM unsharded WHERE branchId = 203622 AND buildNumber <= 113893 AND state = 'FAILED' ORDER BY buildNumber DESC LIMIT 1) AS last_failed) UNION ALL (SELECT * FROM (SELECT * FROM unsharded WHERE branchId = 203622 AND buildNumber <= 113893 AND state = 'SUCCEEDED' ORDER BY buildNumber DESC LIMIT 1) AS last_succeeded) ORDER BY buildNumber DESC LIMIT 1", + "Instructions": { + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "FieldQuery": "select * from (select * from unsharded where 1 != 1) as last_failed where 1 != 1 union all select * from (select * from unsharded where 1 != 1) as last_succeeded where 1 != 1", + "Query": "select * from (select * from unsharded where branchId = 203622 and buildNumber <= 113893 and state = 'FAILED' order by buildNumber desc limit 1) as last_failed union all select * from (select * from unsharded where branchId = 203622 and buildNumber <= 113893 and state = 'SUCCEEDED' order by buildNumber desc limit 1) as last_succeeded order by buildNumber desc limit 1", + "Table": "unsharded" + } + }, + "gen4-plan": { + "QueryType": "SELECT", + "Original": "(SELECT * FROM (SELECT * FROM unsharded WHERE branchId = 203622 AND buildNumber <= 113893 AND state = 'FAILED' ORDER BY buildNumber DESC LIMIT 1) AS last_failed) UNION ALL (SELECT * FROM (SELECT * FROM unsharded WHERE branchId = 203622 AND buildNumber <= 113893 AND state = 'SUCCEEDED' ORDER BY buildNumber DESC LIMIT 1) AS last_succeeded) ORDER BY buildNumber DESC LIMIT 1", + "Instructions": { + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "FieldQuery": "select * from (select * from unsharded where 1 != 1) as last_failed where 1 != 1 union all select * from (select * from unsharded where 1 != 1) as last_succeeded where 1 != 1", + "Query": "select * from (select * from unsharded where branchId = 203622 and buildNumber <= 113893 and state = 'FAILED' order by buildNumber desc limit 1) as last_failed union all select * from (select * from unsharded where branchId = 203622 and buildNumber <= 113893 and state = 'SUCCEEDED' order by buildNumber desc limit 1) as last_succeeded order by buildNumber desc limit 1", + "Table": "unsharded" + }, + "TablesUsed": [ + "main.unsharded" + ] + } } ] diff --git a/go/vt/vtgate/semantics/analyzer.go b/go/vt/vtgate/semantics/analyzer.go index 1375e3914ab..33edf05b68f 100644 --- a/go/vt/vtgate/semantics/analyzer.go +++ b/go/vt/vtgate/semantics/analyzer.go @@ -109,7 +109,7 @@ func (a *analyzer) setError(err error) { switch err := err.(type) { case ProjError: a.projErr = err.Inner - case UnshardedError: + case ShardedError: a.unshardedErr = err.Inner default: if a.inProjection > 0 && vterrors.ErrState(err) == vterrors.NonUniqError { @@ -260,11 +260,11 @@ func (a *analyzer) checkForInvalidConstructs(cursor *sqlparser.Cursor) error { switch node := cursor.Node().(type) { case *sqlparser.Update: if len(node.TableExprs) != 1 { - return UnshardedError{Inner: NewError(UnsupportedMultiTablesInUpdate)} + return ShardedError{Inner: NewError(UnsupportedMultiTablesInUpdate)} } alias, isAlias := node.TableExprs[0].(*sqlparser.AliasedTableExpr) if !isAlias { - return UnshardedError{Inner: NewError(UnsupportedMultiTablesInUpdate)} + return ShardedError{Inner: NewError(UnsupportedMultiTablesInUpdate)} } _, isDerived := alias.Expr.(*sqlparser.DerivedTable) if isDerived { @@ -357,12 +357,12 @@ func (p ProjError) Error() string { return p.Inner.Error() } -// UnshardedError is used to mark an error as something that should only be returned +// ShardedError is used to mark an error as something that should only be returned // if the query is not unsharded -type UnshardedError struct { +type ShardedError struct { Inner error } -func (p UnshardedError) Error() string { +func (p ShardedError) Error() string { return p.Inner.Error() } diff --git a/go/vt/vtgate/semantics/analyzer_test.go b/go/vt/vtgate/semantics/analyzer_test.go index c88c295ac68..96bce0b7edf 100644 --- a/go/vt/vtgate/semantics/analyzer_test.go +++ b/go/vt/vtgate/semantics/analyzer_test.go @@ -124,10 +124,10 @@ func TestBindingSingleTableNegative(t *testing.T) { t.Run(query, func(t *testing.T) { parse, err := sqlparser.Parse(query) require.NoError(t, err) - _, err = Analyze(parse.(sqlparser.SelectStatement), "d", &FakeSI{}) - require.Error(t, err) - require.Contains(t, err.Error(), "symbol") - require.Contains(t, err.Error(), "not found") + st, err := Analyze(parse.(sqlparser.SelectStatement), "d", &FakeSI{}) + require.NoError(t, err) + require.ErrorContains(t, st.NotUnshardedErr, "symbol") + require.ErrorContains(t, st.NotUnshardedErr, "not found") }) } } @@ -144,12 +144,13 @@ func TestBindingSingleAliasedTableNegative(t *testing.T) { t.Run(query, func(t *testing.T) { parse, err := sqlparser.Parse(query) require.NoError(t, err) - _, err = Analyze(parse.(sqlparser.SelectStatement), "", &FakeSI{ + st, err := Analyze(parse.(sqlparser.SelectStatement), "", &FakeSI{ Tables: map[string]*vindexes.Table{ "t": {Name: sqlparser.NewIdentifierCS("t")}, }, }) - require.Error(t, err) + require.NoError(t, err) + require.Error(t, st.NotUnshardedErr) }) } } @@ -310,9 +311,9 @@ func TestMissingTable(t *testing.T) { for _, query := range queries { t.Run(query, func(t *testing.T) { parse, _ := sqlparser.Parse(query) - _, err := Analyze(parse.(sqlparser.SelectStatement), "", &FakeSI{}) - require.Error(t, err) - require.Contains(t, err.Error(), "symbol t.col not found") + st, err := Analyze(parse.(sqlparser.SelectStatement), "", &FakeSI{}) + require.NoError(t, err) + require.ErrorContains(t, st.NotUnshardedErr, "symbol t.col not found") }) } } @@ -470,16 +471,13 @@ func TestScoping(t *testing.T) { t.Run(query.query, func(t *testing.T) { parse, err := sqlparser.Parse(query.query) require.NoError(t, err) - _, err = Analyze(parse.(sqlparser.SelectStatement), "user", &FakeSI{ + st, err := Analyze(parse.(sqlparser.SelectStatement), "user", &FakeSI{ Tables: map[string]*vindexes.Table{ "t": {Name: sqlparser.NewIdentifierCS("t")}, }, }) - if query.errorMessage == "" { - require.NoError(t, err) - } else { - require.EqualError(t, err, query.errorMessage) - } + require.NoError(t, err) + require.EqualError(t, st.NotUnshardedErr, query.errorMessage) }) } } @@ -871,8 +869,9 @@ func TestUnionOrderByRewrite(t *testing.T) { func TestInvalidQueries(t *testing.T) { tcases := []struct { - sql string - err string + sql string + err string + shardedErr string }{{ sql: "select t1.id, t1.col1 from t1 union select t2.uid from t2", err: "The used SELECT statements have a different number of columns", @@ -900,15 +899,37 @@ func TestInvalidQueries(t *testing.T) { }, { sql: "select (select sql_calc_found_rows id from a) as t", err: "Incorrect usage/placement of 'SQL_CALC_FOUND_ROWS'", + }, { + sql: "select id from t1 natural join t2", + err: "VT12001: unsupported: natural join", + }, { + sql: "select * from music where user_id IN (select sql_calc_found_rows * from music limit 10)", + err: "Incorrect usage/placement of 'SQL_CALC_FOUND_ROWS'", + }, { + sql: "select is_free_lock('xyz') from user", + err: "is_free_lock('xyz') allowed only with dual", + }, { + sql: "SELECT * FROM JSON_TABLE('[ {\"c1\": null} ]','$[*]' COLUMNS( c1 INT PATH '$.c1' ERROR ON ERROR )) as jt", + err: "VT12001: unsupported: json_table expressions", + }, { + sql: "select does_not_exist from t1", + shardedErr: "symbol does_not_exist not found", + }, { + sql: "select t1.does_not_exist from t1, t2", + shardedErr: "symbol t1.does_not_exist not found", }} for _, tc := range tcases { t.Run(tc.sql, func(t *testing.T) { parse, err := sqlparser.Parse(tc.sql) require.NoError(t, err) - _, err = Analyze(parse.(sqlparser.SelectStatement), "dbName", fakeSchemaInfo()) - require.Error(t, err) - require.Equal(t, tc.err, err.Error()) + st, err := Analyze(parse.(sqlparser.SelectStatement), "dbName", fakeSchemaInfo()) + if tc.err != "" { + require.EqualError(t, err, tc.err) + } else { + require.NoError(t, err, tc.err) + require.EqualError(t, st.NotUnshardedErr, tc.shardedErr) + } }) } } @@ -1021,9 +1042,13 @@ func TestScopingWDerivedTables(t *testing.T) { "t": {Name: sqlparser.NewIdentifierCS("t")}, }, }) - if query.errorMessage != "" { + + switch { + case query.errorMessage != "" && err != nil: require.EqualError(t, err, query.errorMessage) - } else { + case query.errorMessage != "": + require.EqualError(t, st.NotUnshardedErr, query.errorMessage) + default: require.NoError(t, err) sel := parse.(*sqlparser.Select) assert.Equal(t, query.recursiveExpectation, st.RecursiveDeps(extract(sel, 0)), "RecursiveDeps") diff --git a/go/vt/vtgate/semantics/binder.go b/go/vt/vtgate/semantics/binder.go index 446489928fc..2a1b8c2c0ac 100644 --- a/go/vt/vtgate/semantics/binder.go +++ b/go/vt/vtgate/semantics/binder.go @@ -258,7 +258,7 @@ func (b *binder) resolveColumn(colName *sqlparser.ColName, current *scope, allow } current = current.parent } - return dependency{}, NewError(ColumnNotFound, colName) + return dependency{}, ShardedError{Inner: NewError(ColumnNotFound, colName)} } func (b *binder) resolveColumnInScope(current *scope, expr *sqlparser.ColName, allowMulti bool) (dependencies, error) {