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

vtgate routing cleanup: remove routeOptions #6531

Merged
merged 10 commits into from
Aug 18, 2020
6 changes: 3 additions & 3 deletions go/vt/vtgate/engine/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,7 @@ func (obp OrderbyParams) String() string {
}

// RouteOpcode is a number representing the opcode
// for the Route primitve. Adding new opcodes here
// will require review of the join code and
// the finalizeOptions code in planbuilder.
// for the Route primitve.
type RouteOpcode int

// This is the list of RouteOpcode values.
Expand Down Expand Up @@ -166,6 +164,8 @@ const (
SelectReference
// SelectNone is used for queries that always return empty values
SelectNone
// NumRouteOpcodes is the number of opcodes
NumRouteOpcodes
)

var routeName = map[RouteOpcode]string{
Expand Down
11 changes: 3 additions & 8 deletions go/vt/vtgate/executor_select_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,17 +329,12 @@ func testRowCount(t *testing.T, executor *Executor, wantRowCount int64) {
}

func TestSelectLastInsertIdInUnion(t *testing.T) {
executor, sbc1, _, _ := createLegacyExecutorEnv()
executor, _, _, _ := createLegacyExecutorEnv()
executor.normalize = true
sql := "select last_insert_id() as id union select id from user"
_, err := executorExec(executor, sql, map[string]*querypb.BindVariable{})
require.NoError(t, err)
wantQueries := []*querypb.BoundQuery{{
Sql: "select :__lastInsertId as id from dual union select id from user",
BindVariables: map[string]*querypb.BindVariable{"__lastInsertId": sqltypes.Uint64BindVariable(0)},
}}

assert.Equal(t, wantQueries, sbc1.Queries)
require.Error(t, err)
assert.Contains(t, err.Error(), "unsupported: UNION cannot be executed as a single route")
}

func TestSelectLastInsertIdInWhere(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/planbuilder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ type builder interface {
// info about tables.
type ContextVSchema interface {
FindTable(tablename sqlparser.TableName) (*vindexes.Table, string, topodatapb.TabletType, key.Destination, error)
FindTablesOrVindex(tablename sqlparser.TableName) ([]*vindexes.Table, vindexes.Vindex, string, topodatapb.TabletType, key.Destination, error)
FindTableOrVindex(tablename sqlparser.TableName) (*vindexes.Table, vindexes.Vindex, string, topodatapb.TabletType, key.Destination, error)
DefaultKeyspace() (*vindexes.Keyspace, error)
TargetString() string
Destination() key.Destination
Expand Down
54 changes: 27 additions & 27 deletions go/vt/vtgate/planbuilder/dml.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,76 +101,76 @@ func nameMatch(node sqlparser.Expr, col sqlparser.ColIdent) bool {
}

func buildDMLPlan(vschema ContextVSchema, dmlType string, stmt sqlparser.Statement, tableExprs sqlparser.TableExprs, where *sqlparser.Where, orderBy sqlparser.OrderBy, limit *sqlparser.Limit, comments sqlparser.Comments, nodes ...sqlparser.SQLNode) (*engine.DML, vindexes.SingleColumn, string, error) {
eupd := &engine.DML{}
edml := &engine.DML{}
pb := newPrimitiveBuilder(vschema, newJointab(sqlparser.GetBindvars(stmt)))
ro, err := pb.processDMLTable(tableExprs)
rb, err := pb.processDMLTable(tableExprs)
if err != nil {
return nil, nil, "", err
}
eupd.Keyspace = ro.eroute.Keyspace
if !eupd.Keyspace.Sharded {
edml.Keyspace = rb.eroute.Keyspace
if !edml.Keyspace.Sharded {
// We only validate non-table subexpressions because the previous analysis has already validated them.
var subqueryArgs []sqlparser.SQLNode
subqueryArgs = append(subqueryArgs, nodes...)
subqueryArgs = append(subqueryArgs, where, orderBy, limit)
if !pb.finalizeUnshardedDMLSubqueries(subqueryArgs...) {
return nil, nil, "", vterrors.New(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: sharded subqueries in DML")
}
eupd.Opcode = engine.Unsharded
edml.Opcode = engine.Unsharded
// Generate query after all the analysis. Otherwise table name substitutions for
// routed tables won't happen.
eupd.Query = generateQuery(stmt)
return eupd, nil, "", nil
edml.Query = generateQuery(stmt)
return edml, nil, "", nil
}

if hasSubquery(stmt) {
return nil, nil, "", vterrors.New(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: subqueries in sharded DML")
}

if len(pb.st.tables) != 1 {
return nil, nil, "", vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: multi-table %s statement in sharded keyspace", dmlType)
}

// Generate query after all the analysis. Otherwise table name substitutions for
// routed tables won't happen.
eupd.Query = generateQuery(stmt)
edml.Query = generateQuery(stmt)

directives := sqlparser.ExtractCommentDirectives(comments)
if directives.IsSet(sqlparser.DirectiveMultiShardAutocommit) {
eupd.MultiShardAutocommit = true
edml.MultiShardAutocommit = true
}

eupd.QueryTimeout = queryTimeout(directives)
eupd.Table = ro.vschemaTable
if eupd.Table == nil {
return nil, nil, "", vterrors.New(vtrpcpb.Code_INTERNAL, "internal error: table.vindexTable is mysteriously nil")
edml.QueryTimeout = queryTimeout(directives)

if len(pb.st.tables) != 1 {
return nil, nil, "", vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: multi-table %s statement in sharded keyspace", dmlType)
}
for _, tval := range pb.st.tables {
// There is only one table.
edml.Table = tval.vschemaTable
}

routingType, ksidVindex, ksidCol, vindex, values, err := getDMLRouting(where, eupd.Table)
routingType, ksidVindex, ksidCol, vindex, values, err := getDMLRouting(where, edml.Table)
if err != nil {
return nil, nil, "", err
}

if ro.eroute.TargetDestination != nil {
if ro.eroute.TargetTabletType != topodatapb.TabletType_MASTER {
if rb.eroute.TargetDestination != nil {
if rb.eroute.TargetTabletType != topodatapb.TabletType_MASTER {
return nil, nil, "", vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "unsupported: %s statement with a replica target", dmlType)
}
eupd.Opcode = engine.ByDestination
eupd.TargetDestination = ro.eroute.TargetDestination
return eupd, ksidVindex, ksidCol, nil
edml.Opcode = engine.ByDestination
edml.TargetDestination = rb.eroute.TargetDestination
return edml, ksidVindex, ksidCol, nil
}

eupd.Opcode = routingType
edml.Opcode = routingType
if routingType == engine.Scatter {
if limit != nil {
return nil, nil, "", vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: multi shard %s with limit", dmlType)
}
} else {
eupd.Vindex = vindex
eupd.Values = values
edml.Vindex = vindex
edml.Values = values
}

return eupd, ksidVindex, ksidCol, nil
return edml, ksidVindex, ksidCol, nil
}

func generateDMLSubquery(where *sqlparser.Where, orderBy sqlparser.OrderBy, limit *sqlparser.Limit, table *vindexes.Table, ksidCol string) string {
Expand Down
8 changes: 4 additions & 4 deletions go/vt/vtgate/planbuilder/expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ func hasSubquery(node sqlparser.SQLNode) bool {
func (pb *primitiveBuilder) finalizeUnshardedDMLSubqueries(nodes ...sqlparser.SQLNode) bool {
var keyspace string
if rb, ok := pb.bldr.(*route); ok {
keyspace = rb.routeOptions[0].eroute.Keyspace.Name
keyspace = rb.eroute.Keyspace.Name
} else {
// This code is unreachable because the caller checks.
return false
Expand Down Expand Up @@ -239,11 +239,11 @@ func (pb *primitiveBuilder) finalizeUnshardedDMLSubqueries(nodes ...sqlparser.SQ
samePlan = false
return false, errors.New("dummy")
}
if !innerRoute.removeOptionsWithUnmatchedKeyspace(keyspace) {
if innerRoute.eroute.Keyspace.Name != keyspace {
samePlan = false
return false, errors.New("dummy")
}
for _, sub := range innerRoute.routeOptions[0].substitutions {
for _, sub := range innerRoute.substitutions {
*sub.oldExpr = *sub.newExpr
}
case *sqlparser.Union:
Expand All @@ -260,7 +260,7 @@ func (pb *primitiveBuilder) finalizeUnshardedDMLSubqueries(nodes ...sqlparser.SQ
samePlan = false
return false, errors.New("dummy")
}
if !innerRoute.removeOptionsWithUnmatchedKeyspace(keyspace) {
if innerRoute.eroute.Keyspace.Name != keyspace {
samePlan = false
return false, errors.New("dummy")
}
Expand Down
Loading