Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ func TestSubqueriesExists(t *testing.T) {

mcmp.Exec("insert into t1(id1, id2) values (0,1),(1,2),(2,3),(3,4),(4,5),(5,6)")
mcmp.AssertMatches(`SELECT id2 FROM t1 WHERE EXISTS (SELECT id1 FROM t1 WHERE id1 > 0) ORDER BY id2`, `[[INT64(1)] [INT64(2)] [INT64(3)] [INT64(4)] [INT64(5)] [INT64(6)]]`)
mcmp.AssertMatches(`select * from (select 1) as tmp where exists(select 1 from t1 where id1 = 1)`, `[[INT32(1)]]`)
}

func TestQueryAndSubQWithLimit(t *testing.T) {
Expand Down
26 changes: 22 additions & 4 deletions go/vt/sqlparser/normalizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,11 @@ func Normalize(stmt Statement, reserved *ReservedVars, bindVars map[string]*quer
}

type normalizer struct {
bindVars map[string]*querypb.BindVariable
reserved *ReservedVars
vals map[string]string
err error
bindVars map[string]*querypb.BindVariable
reserved *ReservedVars
vals map[string]string
err error
inDerived bool
}

func newNormalizer(reserved *ReservedVars, bindVars map[string]*querypb.BindVariable) *normalizer {
Expand All @@ -65,8 +66,12 @@ func (nz *normalizer) WalkStatement(cursor *Cursor) bool {
case *Set, *Show, *Begin, *Commit, *Rollback, *Savepoint, *SetTransaction, DDLStatement, *SRollback, *Release, *OtherAdmin, *OtherRead:
return false
case *Select:
_, isDerived := cursor.Parent().(*DerivedTable)
var tmp bool
tmp, nz.inDerived = nz.inDerived, isDerived
_ = Rewrite(node, nz.WalkSelect, nil)
// Don't continue
nz.inDerived = tmp
return false
case *Literal:
nz.convertLiteral(node, cursor)
Expand All @@ -85,6 +90,19 @@ func (nz *normalizer) WalkStatement(cursor *Cursor) bool {
// WalkSelect normalizes the AST in Select mode.
func (nz *normalizer) WalkSelect(cursor *Cursor) bool {
switch node := cursor.Node().(type) {
case *Select:
_, isDerived := cursor.Parent().(*DerivedTable)
if !isDerived {
return true
}
var tmp bool
tmp, nz.inDerived = nz.inDerived, isDerived
_ = Rewrite(node, nz.WalkSelect, nil)
// Don't continue
nz.inDerived = tmp
return false
case SelectExprs:
return !nz.inDerived
case *Literal:
nz.convertLiteralDedup(node, cursor)
case *ComparisonExpr:
Expand Down
8 changes: 8 additions & 0 deletions go/vt/sqlparser/normalizer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,14 @@ func TestNormalize(t *testing.T) {
outbv: map[string]*querypb.BindVariable{
"bv1": sqltypes.ValueBindVariable(sqltypes.MakeTrusted(sqltypes.Datetime, []byte("2022-08-06 17:05:12"))),
},
}, {
// we don't want to replace literals on the select expressions of a derived table
// these expressions can be referenced from the outside,
// and changing them to bindvars can change the meaning of the query
// example of problematic query: select tmp.`1` from (select 1) as tmp
in: `select * from (select 12) as t`,
outstmt: `select * from (select 12 from dual) as t`,
outbv: map[string]*querypb.BindVariable{},
}}
for _, tc := range testcases {
t.Run(tc.in, func(t *testing.T) {
Expand Down
9 changes: 5 additions & 4 deletions go/vt/vtgate/planbuilder/abstract/derived.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@ limitations under the License.
package abstract

import (
vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
"vitess.io/vitess/go/vt/sqlparser"
"vitess.io/vitess/go/vt/vterrors"
"vitess.io/vitess/go/vt/vtgate/semantics"
)

Expand All @@ -44,8 +42,11 @@ func (d *Derived) TableID() semantics.TableSet {
func (d *Derived) PushPredicate(expr sqlparser.Expr, semTable *semantics.SemTable) (LogicalOperator, error) {
tableInfo, err := semTable.TableInfoForExpr(expr)
if err != nil {
if err == semantics.ErrMultipleTables {
return nil, semantics.ProjError{Inner: vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: unable to split predicates to derived table: %s", sqlparser.String(expr))}
if err == semantics.ErrNotSingleTable {
return &Filter{
Source: d,
Predicates: []sqlparser.Expr{expr},
}, nil
}
return nil, err
}
Expand Down
7 changes: 5 additions & 2 deletions go/vt/vtgate/planbuilder/physical/operator_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,11 @@ func PushPredicate(ctx *plancontext.PlanningContext, expr sqlparser.Expr, op abs
case *Derived:
tableInfo, err := ctx.SemTable.TableInfoForExpr(expr)
if err != nil {
if err == semantics.ErrMultipleTables {
return nil, semantics.ProjError{Inner: vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: unable to split predicates to derived table: %s", sqlparser.String(expr))}
if err == semantics.ErrNotSingleTable {
return &Filter{
Source: op,
Predicates: []sqlparser.Expr{expr},
}, nil
}
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/planbuilder/projection_pushing.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ func addExpressionToRoute(ctx *plancontext.PlanningContext, rb *routeGen4, expr

func rewriteProjectionOfDerivedTable(expr *sqlparser.AliasedExpr, semTable *semantics.SemTable) error {
ti, err := semTable.TableInfoForExpr(expr.Expr)
if err != nil && err != semantics.ErrMultipleTables {
if err != nil && err != semantics.ErrNotSingleTable {
return err
}
_, isDerivedTable := ti.(*semantics.DerivedTable)
Expand Down
21 changes: 20 additions & 1 deletion go/vt/vtgate/planbuilder/testdata/dml_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -3827,7 +3827,26 @@
"main.user_privacy_consents"
]
},
"gen4-plan": "unsupported: unable to split predicates to derived table: not :__sq_has_values1"
"gen4-plan": {
"QueryType": "INSERT",
"Original": "INSERT INTO main.user_privacy_consents (user_id, accepted_at) SELECT user_id, accepted_at FROM (SELECT 1 as user_id, 1629194864 as accepted_at) AS tmp WHERE NOT EXISTS (SELECT user_id FROM main.user_privacy_consents WHERE user_id = 1)",
"Instructions": {
"OperatorType": "Insert",
"Variant": "Unsharded",
"Keyspace": {
"Name": "main",
"Sharded": false
},
"TargetTabletType": "PRIMARY",
"MultiShardAutocommit": false,
"Query": "insert into user_privacy_consents(user_id, accepted_at) select user_id, accepted_at from (select 1 as user_id, 1629194864 as accepted_at from dual) as tmp where not exists (select 1 from user_privacy_consents where user_id = 1 limit 1) for update",
"TableName": "user_privacy_consents"
},
"TablesUsed": [
"main.dual",
"main.user_privacy_consents"
]
}
},
{
"comment": "Delete on backfilling unique lookup vindex should be a scatter",
Expand Down
3 changes: 1 addition & 2 deletions go/vt/vtgate/planbuilder/testdata/unsupported_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -400,8 +400,7 @@
{
"comment": "outer and inner subquery route reference the same \"uu.id\" name\n# but they refer to different things. The first reference is to the outermost query,\n# and the second reference is to the innermost 'from' subquery.\n# This query will never work as the inner derived table is only selecting one of the column",
"query": "select id2 from user uu where id in (select id from user where id = uu.id and user.col in (select col from (select id from user_extra where user_id = 5) uu where uu.user_id = uu.id))",
"v3-plan": "unsupported: cross-shard correlated subquery",
"gen4-plan": "unsupported: unable to split predicates to derived table: uu.user_id = uu.id"
"plan": "unsupported: cross-shard correlated subquery"
},
{
"comment": "outer and inner subquery route reference the same \"uu.id\" name\n# but they refer to different things. The first reference is to the outermost query,\n# and the second reference is to the innermost 'from' subquery.\n# changed to project all the columns from the derived tables.",
Expand Down
3 changes: 3 additions & 0 deletions go/vt/vtgate/semantics/early_rewriter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ func TestExpandStar(t *testing.T) {
}, {
sql: "select 1 from t1 join t5 using (b) having b = 12",
expSQL: "select 1 from t1 join t5 where t1.b = t5.b having t1.b = 12",
}, {
sql: "select * from (select 12) as t",
expSQL: "select t.`12` from (select 12 from dual) as t",
}}
for _, tcase := range tcases {
t.Run(tcase.sql, func(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions go/vt/vtgate/semantics/semantic_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ type (
)

var (
// ErrMultipleTables refers to an error happening when something should be used only for single tables
ErrMultipleTables = vterrors.Errorf(vtrpcpb.Code_INTERNAL, "[BUG] should only be used for single tables")
// ErrNotSingleTable refers to an error happening when something should be used only for single tables
ErrNotSingleTable = vterrors.Errorf(vtrpcpb.Code_INTERNAL, "[BUG] should only be used for single tables")
)

// CopyDependencies copies the dependencies from one expression into the other
Expand Down Expand Up @@ -169,7 +169,7 @@ func (st *SemTable) ReplaceTableSetFor(id TableSet, t *sqlparser.AliasedTableExp
func (st *SemTable) TableInfoFor(id TableSet) (TableInfo, error) {
offset := id.TableOffset()
if offset < 0 {
return nil, ErrMultipleTables
return nil, ErrNotSingleTable
}
return st.Tables[offset], nil
}
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/semantics/table_collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func (tc *tableCollector) tableSetFor(t *sqlparser.AliasedTableExpr) TableSet {
func (tc *tableCollector) tableInfoFor(id TableSet) (TableInfo, error) {
offset := id.TableOffset()
if offset < 0 {
return nil, ErrMultipleTables
return nil, ErrNotSingleTable
}
return tc.Tables[offset], nil
}
Expand Down