[planner fix] make unknown column an error only for sharded queries (#12704) (#12726)

Signed-off-by: Andres Taylor <andres@planetscale.com>
This commit is contained in:
Andres Taylor 2023-03-27 13:50:51 +02:00 коммит произвёл GitHub
Родитель ca73cabc44
Коммит ddeabc7598
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
6 изменённых файлов: 176 добавлений и 34 удалений

Просмотреть файл

@ -4588,7 +4588,25 @@
{
"comment": "verify ',' vs JOIN precedence",
"query": "select u1.a from unsharded u1, unsharded u2 join unsharded u3 on u1.a = u2.a",
"plan": "symbol u1.a not found"
"v3-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)",
@ -4598,7 +4616,25 @@
{
"comment": "table names should be case-sensitive",
"query": "select unsharded.id from unsharded where Unsharded.val = 1",
"plan": "symbol Unsharded.val not found"
"v3-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",
@ -6207,5 +6243,49 @@
"user.user"
]
}
},
{
"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": "symbol missing_column not found",
"gen4-plan": "Column 'missing_column' in field list is ambiguous"
}
]

Просмотреть файл

@ -37,7 +37,7 @@
"Table": "information_schema.a, information_schema.b"
}
},
"gen4-plan": "symbol a.id not found"
"gen4-plan": "symbol b.id not found"
},
{
"comment": "information schema query that uses table_schema",
@ -382,7 +382,7 @@
]
}
},
"gen4-plan": "symbol KCU.DELETE_RULE not found"
"gen4-plan": "symbol S.TABLE_NAME not found"
},
{
"comment": "information_schema.routines",
@ -1182,7 +1182,7 @@
"Table": "INFORMATION_SCHEMA.`TABLES`"
}
},
"gen4-plan": "symbol other_column not found"
"gen4-plan": "symbol foobar not found"
},
{
"comment": "expand star with information schema",

Просмотреть файл

@ -2342,5 +2342,42 @@
"user.user"
]
}
},
{
"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"
]
}
}
]

Просмотреть файл

@ -105,7 +105,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 {
@ -256,11 +256,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: vterrors.New(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: multiple tables in update")}
return ShardedError{Inner: vterrors.New(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: multiple tables in update")}
}
alias, isAlias := node.TableExprs[0].(*sqlparser.AliasedTableExpr)
if !isAlias {
return UnshardedError{Inner: vterrors.New(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: multiple tables in update")}
return ShardedError{Inner: vterrors.New(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: multiple tables in update")}
}
_, isDerived := alias.Expr.(*sqlparser.DerivedTable)
if isDerived {
@ -353,12 +353,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()
}

Просмотреть файл

@ -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: "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: "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")

Просмотреть файл

@ -260,7 +260,7 @@ func (b *binder) resolveColumn(colName *sqlparser.ColName, current *scope, allow
}
current = current.parent
}
return dependency{}, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.BadFieldError, "symbol %s not found", sqlparser.String(colName))
return dependency{}, ShardedError{Inner: vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.BadFieldError, "symbol %s not found", sqlparser.String(colName))}
}
func (b *binder) resolveColumnInScope(current *scope, expr *sqlparser.ColName, allowMulti bool) (dependencies, error) {