Skip to content

Commit 6d8a073

Browse files
[release-22.0] fix: remove database qualifier after building query in operator to sql (vitessio#18602) (vitessio#18605)
Signed-off-by: Harshit Gangal <harshit@planetscale.com> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com>
1 parent 3a28c0c commit 6d8a073

File tree

4 files changed

+55
-17
lines changed

4 files changed

+55
-17
lines changed

go/test/endtoend/vtgate/queries/orderby/orderby_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,15 @@ func TestSimpleOrderBy(t *testing.T) {
5353
mcmp.AssertMatches(`SELECT id2 FROM t1 ORDER BY id2 ASC`, `[[INT64(5)] [INT64(6)] [INT64(7)] [INT64(8)] [INT64(9)] [INT64(10)]]`)
5454
}
5555

56+
// TestQueryWithDBQualifier tests that we remove the db qualifier in the plan output that is sent down to the database.
57+
func TestQueryWithDBQualifier(t *testing.T) {
58+
mcmp, closer := start(t)
59+
defer closer()
60+
61+
mcmp.Exec("insert into t1(id1, id2) values (0,10),(1,9),(2,8),(3,7),(4,6),(5,5)")
62+
mcmp.Exec(`SELECT ks_orderby.t1.id1, ks_orderby.t1.id2 FROM ks_orderby.t1 ORDER BY ks_orderby.t1.id2 ASC, ks_orderby.t1.id1 desc`)
63+
}
64+
5665
func TestOrderBy(t *testing.T) {
5766
mcmp, closer := start(t)
5867
defer closer()

go/vt/vtgate/planbuilder/operators/SQL_builder.go

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ func ToSQL(ctx *plancontext.PlanningContext, op Operator) (_ sqlparser.Statement
5555
if ctx.SemTable != nil {
5656
q.sortTables()
5757
}
58+
sqlparser.RemoveKeyspaceIgnoreSysSchema(q.stmt)
5859
return q.stmt, q.dmlOperator, nil
5960
}
6061

@@ -395,15 +396,6 @@ func (ts *tableSorter) Swap(i, j int) {
395396
ts.sel.From[i], ts.sel.From[j] = ts.sel.From[j], ts.sel.From[i]
396397
}
397398

398-
func removeKeyspaceFromSelectExpr(expr sqlparser.SelectExpr) {
399-
switch expr := expr.(type) {
400-
case *sqlparser.AliasedExpr:
401-
sqlparser.RemoveKeyspaceInCol(expr.Expr)
402-
case *sqlparser.StarExpr:
403-
expr.TableName.Qualifier = sqlparser.NewIdentifierCS("")
404-
}
405-
}
406-
407399
func stripDownQuery(from, to sqlparser.TableStatement) {
408400
switch node := from.(type) {
409401
case *sqlparser.Select:
@@ -418,9 +410,6 @@ func stripDownQuery(from, to sqlparser.TableStatement) {
418410
toNode.Comments = node.Comments
419411
toNode.Limit = node.Limit
420412
toNode.SelectExprs = node.SelectExprs
421-
for _, expr := range toNode.SelectExprs.Exprs {
422-
removeKeyspaceFromSelectExpr(expr)
423-
}
424413
case *sqlparser.Union:
425414
toNode, ok := to.(*sqlparser.Union)
426415
if !ok {
@@ -666,8 +655,6 @@ func buildFilter(op *Filter, qb *queryBuilder) {
666655
func buildDerived(op *Horizon, qb *queryBuilder) {
667656
buildQuery(op.Source, qb)
668657

669-
sqlparser.RemoveKeyspaceInCol(op.Query)
670-
671658
stmt := qb.stmt
672659
qb.stmt = nil
673660
switch sel := stmt.(type) {
@@ -718,7 +705,6 @@ func buildDerivedSelect(op *Horizon, qb *queryBuilder, sel *sqlparser.Select) {
718705
func buildHorizon(op *Horizon, qb *queryBuilder) {
719706
buildQuery(op.Source, qb)
720707
stripDownQuery(op.Query, qb.asSelectStatement())
721-
sqlparser.RemoveKeyspaceInCol(qb.stmt)
722708
}
723709

724710
func buildRecursiveCTE(op *RecurseCTE, qb *queryBuilder) {

go/vt/vtgate/planbuilder/operators/route.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -567,8 +567,6 @@ func createProjection(ctx *plancontext.PlanningContext, src Operator, derivedNam
567567
}
568568

569569
func (r *Route) AddColumn(ctx *plancontext.PlanningContext, reuse bool, gb bool, expr *sqlparser.AliasedExpr) int {
570-
removeKeyspaceFromSelectExpr(expr)
571-
572570
if reuse {
573571
offset := r.FindCol(ctx, expr.Expr, true)
574572
if offset != -1 {

go/vt/vtgate/planbuilder/testdata/from_cases.json

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5074,5 +5074,50 @@
50745074
"user.user"
50755075
]
50765076
}
5077+
},
5078+
{
5079+
"comment": "order by and project pushed under route having database qualifier - it should be removed in final query",
5080+
"query": "select user.user.col from user.user order by user.user.id",
5081+
"plan": {
5082+
"Type": "Scatter",
5083+
"QueryType": "SELECT",
5084+
"Original": "select user.user.col from user.user order by user.user.id",
5085+
"Instructions": {
5086+
"OperatorType": "Route",
5087+
"Variant": "Scatter",
5088+
"Keyspace": {
5089+
"Name": "user",
5090+
"Sharded": true
5091+
},
5092+
"FieldQuery": "select `user`.col, `user`.id, weight_string(`user`.id) from `user` where 1 != 1",
5093+
"OrderBy": "(1|2) ASC",
5094+
"Query": "select `user`.col, `user`.id, weight_string(`user`.id) from `user` order by `user`.id asc",
5095+
"ResultColumns": 1,
5096+
"Table": "`user`"
5097+
},
5098+
"TablesUsed": [
5099+
"user.user"
5100+
]
5101+
}
5102+
},
5103+
{
5104+
"comment": "order by and project pushed under route having information_schema database qualifier - it should not be removed in final query",
5105+
"query": "select information_schema.table.col from information_schema.table order by information_schema.table.name",
5106+
"plan": {
5107+
"Type": "Passthrough",
5108+
"QueryType": "SELECT",
5109+
"Original": "select information_schema.table.col from information_schema.table order by information_schema.table.name",
5110+
"Instructions": {
5111+
"OperatorType": "Route",
5112+
"Variant": "DBA",
5113+
"Keyspace": {
5114+
"Name": "main",
5115+
"Sharded": false
5116+
},
5117+
"FieldQuery": "select information_schema.`table`.col from information_schema.`table` where 1 != 1",
5118+
"Query": "select information_schema.`table`.col from information_schema.`table` order by information_schema.`table`.`name` asc",
5119+
"Table": "information_schema.`table`"
5120+
}
5121+
}
50775122
}
50785123
]

0 commit comments

Comments
 (0)