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

Remove keyspace from query before sending it on #9190

Merged
merged 2 commits into from
Nov 11, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions go/vt/sqlparser/ast_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -1611,3 +1611,16 @@ func defaultRequiresParens(ct *ColumnType) bool {

return true
}

// RemoveKeyspaceFromColName removes the Qualifier.Qualifier on all ColNames in the expression tree
func RemoveKeyspaceFromColName(expr Expr) Expr {
return 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
}).(Expr) // This hard cast is safe because we do not change the type the input
}
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)
systay marked this conversation as resolved.
Show resolved Hide resolved
expr.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