Skip to content

Commit

Permalink
Merge pull request vitessio#9190 from planetscale/fix-9135
Browse files Browse the repository at this point in the history
Remove keyspace from query before sending it on
  • Loading branch information
systay authored Nov 11, 2021
2 parents c88bcef + 1389cba commit b04bca6
Show file tree
Hide file tree
Showing 9 changed files with 193 additions and 23 deletions.
15 changes: 15 additions & 0 deletions go/vt/sqlparser/ast_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -1611,3 +1611,18 @@ func defaultRequiresParens(ct *ColumnType) bool {

return true
}

// RemoveKeyspaceFromColName removes the Qualifier.Qualifier on all ColNames in the expression tree
func RemoveKeyspaceFromColName(expr Expr) Expr {
Rewrite(expr, nil, func(cursor *Cursor) bool {
switch col := cursor.Node().(type) {
case *ColName:
if !col.Qualifier.Qualifier.IsEmpty() {
col.Qualifier.Qualifier = NewTableIdent("")
}
}
return true
}) // This hard cast is safe because we do not change the type the input

return expr
}
6 changes: 3 additions & 3 deletions go/vt/vtgate/planbuilder/abstract/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func getOperatorFromTableExpr(tableExpr sqlparser.TableExpr, semTable *semantics
}
op := createJoin(lhs, rhs)
if tableExpr.Condition.On != nil {
err = op.PushPredicate(tableExpr.Condition.On, semTable)
err = op.PushPredicate(sqlparser.RemoveKeyspaceFromColName(tableExpr.Condition.On), semTable)
if err != nil {
return nil, err
}
Expand All @@ -108,7 +108,7 @@ func getOperatorFromTableExpr(tableExpr sqlparser.TableExpr, semTable *semantics
if tableExpr.Join == sqlparser.RightJoinType {
lhs, rhs = rhs, lhs
}
return &Join{LHS: lhs, RHS: rhs, LeftJoin: true, Predicate: tableExpr.Condition.On}, nil
return &Join{LHS: lhs, RHS: rhs, LeftJoin: true, Predicate: sqlparser.RemoveKeyspaceFromColName(tableExpr.Condition.On)}, nil
default:
return nil, vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: %s", tableExpr.Join.ToString())
}
Expand Down Expand Up @@ -207,7 +207,7 @@ func createOperatorFromSelect(sel *sqlparser.Select, semTable *semantics.SemTabl
if sel.Where != nil {
exprs := sqlparser.SplitAndExpression(nil, sel.Where.Expr)
for _, expr := range exprs {
err := op.PushPredicate(expr, semTable)
err := op.PushPredicate(sqlparser.RemoveKeyspaceFromColName(expr), semTable)
if err != nil {
return nil, err
}
Expand Down
7 changes: 4 additions & 3 deletions go/vt/vtgate/planbuilder/abstract/operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (lcr *lineCountingReader) nextLine() (string, error) {
func readTestCase(lcr *lineCountingReader) (testCase, error) {
query := ""
var err error
for query == "" || query == "\n" {
for query == "" || query == "\n" || strings.HasPrefix(query, "#") {
query, err = lcr.nextLine()
if err != nil {
return testCase{}, err
Expand All @@ -63,9 +63,9 @@ func readTestCase(lcr *lineCountingReader) (testCase, error) {
jsonPart, err := lcr.nextLine()
if err != nil {
if err == io.EOF {
return testCase{}, fmt.Errorf("test data is bad. expectation not finished")
return tc, fmt.Errorf("test data is bad. expectation not finished")
}
return testCase{}, err
return tc, err
}
if jsonPart == "}\n" {
tc.expected += "}"
Expand Down Expand Up @@ -95,6 +95,7 @@ func TestOperator(t *testing.T) {
break
}
t.Run(fmt.Sprintf("%d:%s", tc.line, tc.query), func(t *testing.T) {
require.NoError(t, err)
tree, err := sqlparser.Parse(tc.query)
require.NoError(t, err)
stmt := tree.(sqlparser.SelectStatement)
Expand Down
29 changes: 29 additions & 0 deletions go/vt/vtgate/planbuilder/abstract/operator_test_data.txt
Original file line number Diff line number Diff line change
Expand Up @@ -415,3 +415,32 @@ SubQuery: {
TableSet{0}:`user` where exists (select user_id from user_extra where user_id = 3 and user_id < `user`.id)
}
}

# we should remove the keyspace from predicates
select ks.tbl.col from ks.tbl where ks.tbl.id = 1
QueryGraph: {
Tables:
TableSet{0}:ks.tbl where tbl.id = 1
}

select 1 from ks.t join ks.y on ks.t.id = ks.y.t_id
QueryGraph: {
Tables:
TableSet{0}:ks.t
TableSet{1}:ks.y
JoinPredicates:
TableSet{0,1} - t.id = y.t_id
}

select 1 from ks.t left join ks.y on ks.t.id = ks.y.t_id
OuterJoin: {
Inner: QueryGraph: {
Tables:
TableSet{0}:ks.t
}
Outer: QueryGraph: {
Tables:
TableSet{1}:ks.y
}
Predicate: t.id = y.t_id
}
11 changes: 1 addition & 10 deletions go/vt/vtgate/planbuilder/horizon_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func pushProjection(expr *sqlparser.AliasedExpr, plan logicalPlan, semTable *sem
return i, false, nil
}
}
expr = removeKeyspaceFromColName(expr)
expr.Expr = sqlparser.RemoveKeyspaceFromColName(expr.Expr)
sel, isSel := node.Select.(*sqlparser.Select)
if !isSel {
return 0, false, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.BadFieldError, "Unknown column '%s' in 'order clause'", sqlparser.String(expr))
Expand Down Expand Up @@ -348,15 +348,6 @@ func rewriteProjectionOfDerivedTable(expr *sqlparser.AliasedExpr, semTable *sema
return nil
}

func removeKeyspaceFromColName(expr *sqlparser.AliasedExpr) *sqlparser.AliasedExpr {
if _, ok := expr.Expr.(*sqlparser.ColName); ok {
expr = sqlparser.CloneRefOfAliasedExpr(expr)
col := expr.Expr.(*sqlparser.ColName)
col.Qualifier.Qualifier = sqlparser.NewTableIdent("")
}
return expr
}

func checkIfAlreadyExists(expr *sqlparser.AliasedExpr, node sqlparser.SelectStatement, semTable *semantics.SemTable) int {
exprDep := semTable.RecursiveDeps(expr.Expr)
// Here to find if the expr already exists in the SelectStatement, we have 3 cases
Expand Down
12 changes: 6 additions & 6 deletions go/vt/vtgate/planbuilder/route_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,18 +416,18 @@ func planSingleShardRoutePlan(sel sqlparser.SelectStatement, rb *route) error {
return err
}
sqlparser.Rewrite(rb.Select, func(cursor *sqlparser.Cursor) bool {
if aliasedExpr, ok := cursor.Node().(*sqlparser.AliasedExpr); ok {
cursor.Replace(removeKeyspaceFromColName(aliasedExpr))
if aliasedExpr, ok := cursor.Node().(sqlparser.SelectExpr); ok {
removeKeyspaceFromSelectExpr(aliasedExpr)
}
return true
}, nil)
return nil
}

func removeKeyspaceFromSelectExpr(expr sqlparser.SelectExpr, ast *sqlparser.Select, i int) {
func removeKeyspaceFromSelectExpr(expr sqlparser.SelectExpr) {
switch expr := expr.(type) {
case *sqlparser.AliasedExpr:
ast.SelectExprs[i] = removeKeyspaceFromColName(expr)
sqlparser.RemoveKeyspaceFromColName(expr.Expr)
case *sqlparser.StarExpr:
expr.TableName.Qualifier = sqlparser.NewTableIdent("")
}
Expand All @@ -448,8 +448,8 @@ func stripDownQuery(from, to sqlparser.SelectStatement) error {
toNode.OrderBy = node.OrderBy
toNode.Comments = node.Comments
toNode.SelectExprs = node.SelectExprs
for i, expr := range toNode.SelectExprs {
removeKeyspaceFromSelectExpr(expr, toNode, i)
for _, expr := range toNode.SelectExprs {
removeKeyspaceFromSelectExpr(expr)
}
case *sqlparser.Union:
toNode, ok := to.(*sqlparser.Union)
Expand Down
117 changes: 117 additions & 0 deletions go/vt/vtgate/planbuilder/testdata/from_cases.txt
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,123 @@ Gen4 plan same as above
"table disabled has been disabled"
Gen4 plan same as above

"select second_user.foo.col from second_user.foo join user on second_user.foo.id = user.id where second_user.foo.col = 42"
{
"QueryType": "SELECT",
"Original": "select second_user.foo.col from second_user.foo join user on second_user.foo.id = user.id where second_user.foo.col = 42",
"Instructions": {
"OperatorType": "Route",
"Variant": "SelectScatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select foo.col from `user` as foo join `user` on foo.id = `user`.id where 1 != 1",
"Query": "select foo.col from `user` as foo join `user` on foo.id = `user`.id where foo.col = 42",
"Table": "`user`"
}
}
{
"QueryType": "SELECT",
"Original": "select second_user.foo.col from second_user.foo join user on second_user.foo.id = user.id where second_user.foo.col = 42",
"Instructions": {
"OperatorType": "Route",
"Variant": "SelectScatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select foo.col from `user` as foo, `user` where 1 != 1",
"Query": "select foo.col from `user` as foo, `user` where foo.col = 42 and foo.id = `user`.id",
"Table": "`user`"
}
}

"select user.music.foo from user.music join user on user.music.id = user.id where user.music.col = 42"
{
"QueryType": "SELECT",
"Original": "select user.music.foo from user.music join user on user.music.id = user.id where user.music.col = 42",
"Instructions": {
"OperatorType": "Join",
"Variant": "Join",
"JoinColumnIndexes": "-1",
"JoinVars": {
"music_id": 1
},
"TableName": "music_`user`",
"Inputs": [
{
"OperatorType": "Route",
"Variant": "SelectScatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select music.foo, music.id from music where 1 != 1",
"Query": "select music.foo, music.id from music where music.col = 42",
"Table": "music"
},
{
"OperatorType": "Route",
"Variant": "SelectEqualUnique",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select 1 from `user` where 1 != 1",
"Query": "select 1 from `user` where `user`.id = :music_id",
"Table": "`user`",
"Values": [
":music_id"
],
"Vindex": "user_index"
}
]
}
}
{
"QueryType": "SELECT",
"Original": "select user.music.foo from user.music join user on user.music.id = user.id where user.music.col = 42",
"Instructions": {
"OperatorType": "Join",
"Variant": "Join",
"JoinColumnIndexes": "-2",
"JoinVars": {
"music_id": 0
},
"TableName": "music_`user`",
"Inputs": [
{
"OperatorType": "Route",
"Variant": "SelectScatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select music.id, music.foo from music where 1 != 1",
"Query": "select music.id, music.foo from music where music.col = 42",
"Table": "music"
},
{
"OperatorType": "Route",
"Variant": "SelectEqualUnique",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select 1 from `user` where 1 != 1",
"Query": "select 1 from `user` where `user`.id = :music_id",
"Table": "`user`",
"Values": [
":music_id"
],
"Vindex": "user_index"
}
]
}
}


# ',' join
"select music.col from user, music"
{
Expand Down
3 changes: 3 additions & 0 deletions go/vt/vtgate/planbuilder/testdata/schema_test.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
}, {
"from_table": "second_user.user",
"to_tables": ["user.user"]
}, {
"from_table": "second_user.foo",
"to_tables": ["user.user"]
}, {
"from_table": "primary_redirect@primary",
"to_tables": ["user.user"]
Expand Down
16 changes: 15 additions & 1 deletion go/vt/vtgate/planbuilder/testdata/systemtables_cases.txt
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,21 @@ Gen4 plan same as above
"Table": "information_schema.a"
}
}
Gen4 plan same as above
{
"QueryType": "SELECT",
"Original": "select * from information_schema.a where information_schema.a.b=10",
"Instructions": {
"OperatorType": "Route",
"Variant": "SelectDBA",
"Keyspace": {
"Name": "main",
"Sharded": false
},
"FieldQuery": "select * from information_schema.a where 1 != 1",
"Query": "select * from information_schema.a where a.b = 10",
"Table": "information_schema.a"
}
}

# union of information_schema
"select * from information_schema.a union select * from information_schema.b"
Expand Down

0 comments on commit b04bca6

Please sign in to comment.