Skip to content

Commit

Permalink
update and delete having vindex changes be classified as multishard a…
Browse files Browse the repository at this point in the history 
…nd using lookup vindex as lookup plan type

Signed-off-by: Harshit Gangal <[email protected]>
  • Loading branch information
@harshit-gangal
harshit-gangal committed Feb 11, 2025
1 parent c7ec2af commit 329f5be
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 44 deletions.
6 changes: 5 additions & 1 deletion go/vt/vtgate/engine/delete.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func (del *Delete) GetFields(context.Context, VCursor, map[string]*querypb.BindV
// Note: the commit order may be different from the DML order because it's possible
// for DMLs to reuse existing transactions.
func (del *Delete) deleteVindexEntries(ctx context.Context, vcursor VCursor, bindVars map[string]*querypb.BindVariable, rss []*srvtopo.ResolvedShard) error {
if del.OwnedVindexQuery == "" {
if !del.isVindexModified() {
return nil
}
queries := make([]*querypb.BoundQuery, len(rss))
Expand Down Expand Up @@ -120,6 +120,10 @@ func (del *Delete) deleteVindexEntries(ctx context.Context, vcursor VCursor, bin
return nil
}

func (del *Delete) isVindexModified() bool {
return del.OwnedVindexQuery != ""
}

func (del *Delete) description() PrimitiveDescription {
other := map[string]any{
"Query": del.Query,
Expand Down
41 changes: 29 additions & 12 deletions go/vt/vtgate/engine/plan.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ const (
PlanLocal
PlanPassthrough
PlanMultiShard
PlanScatter
PlanLookup
PlanScatter
PlanJoinOp
PlanForeignKey
PlanComplex
Expand All @@ -69,6 +69,13 @@ const (
PlanTransaction
)

func higher(a, b PlanType) PlanType {
if a > b {
return a
}
return b
}

func NewPlan(query string, stmt sqlparser.Statement, primitive Primitive, bindVarNeeds *sqlparser.BindVarNeeds, tablesUsed []string) *Plan {
return &Plan{
Type: getPlanType(primitive),
Expand Down Expand Up @@ -173,9 +180,17 @@ func getPlanType(p Primitive) PlanType {
case *Route:
return getPlanTypeFromRoutingParams(prim.RoutingParameters)
case *Update:
return getPlanTypeFromRoutingParams(prim.RoutingParameters)
pt := getPlanTypeFromRoutingParams(prim.RoutingParameters)
if prim.isVindexModified() {
return higher(pt, PlanMultiShard)
}
return pt
case *Delete:
return getPlanTypeFromRoutingParams(prim.RoutingParameters)
pt := getPlanTypeFromRoutingParams(prim.RoutingParameters)
if prim.isVindexModified() {
return higher(pt, PlanMultiShard)
}
return pt
case *Insert:
if prim.Opcode == InsertUnsharded {
return PlanPassthrough
Expand Down Expand Up @@ -215,9 +230,17 @@ func getPlanTypeFromRoutingParams(rp *RoutingParameters) PlanType {
panic("RoutingParameters is nil, cannot determine plan type")
}
switch rp.Opcode {
case Unsharded, EqualUnique, Next, DBA, Reference:
case Unsharded, Next, DBA, Reference:
return PlanPassthrough
case EqualUnique:
if rp.Vindex != nil && rp.Vindex.NeedsVCursor() {
return PlanLookup
}
return PlanPassthrough
case Equal, IN, Between, MultiEqual, SubShard, ByDestination:
if rp.Vindex != nil && rp.Vindex.NeedsVCursor() {
return PlanLookup
}
return PlanMultiShard
case Scatter:
return PlanScatter
Expand All @@ -230,14 +253,8 @@ func getPlanTypeFromRoutingParams(rp *RoutingParameters) PlanType {
func getPlanTypeForUpsert(prim *Upsert) PlanType {
var finalPlanType PlanType
for _, u := range prim.Upserts {
pt := getPlanType(u.Update)
if pt > finalPlanType {
finalPlanType = pt
}
pt = getPlanType(u.Insert)
if pt > finalPlanType {
finalPlanType = pt
}
finalPlanType = higher(finalPlanType, getPlanType(u.Update))
finalPlanType = higher(finalPlanType, getPlanType(u.Insert))
}
return finalPlanType
}
Expand Down
6 changes: 5 additions & 1 deletion go/vt/vtgate/engine/update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (upd *Update) GetFields(ctx context.Context, vcursor VCursor, bindVars map[
// Note 2: While changes are being committed, the changing row could be
// unreachable by either the new or old column values.
func (upd *Update) updateVindexEntries(ctx context.Context, vcursor VCursor, bindVars map[string]*querypb.BindVariable, rss []*srvtopo.ResolvedShard) error {
if len(upd.ChangedVindexValues) == 0 {
if !upd.isVindexModified() {
return nil
}
queries := make([]*querypb.BoundQuery, len(rss))
Expand Down Expand Up @@ -194,6 +194,10 @@ func (upd *Update) updateVindexEntries(ctx context.Context, vcursor VCursor, bin
return nil
}

func (upd *Update) isVindexModified() bool {
return len(upd.ChangedVindexValues) != 0
}

func (upd *Update) description() PrimitiveDescription {
other := map[string]any{
"Query": upd.Query,
Expand Down
58 changes: 29 additions & 29 deletions go/vt/vtgate/planbuilder/testdata/dml_cases.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@
"comment": "update by primary keyspace id, changing one vindex column",
"query": "update user_metadata set email = '[email protected]' where user_id = 1",
"plan": {
"Type": "Passthrough",
"Type": "MultiShard",
"QueryType": "UPDATE",
"Original": "update user_metadata set email = '[email protected]' where user_id = 1",
"Instructions": {
Expand Down Expand Up @@ -436,7 +436,7 @@
"comment": "update by primary keyspace id, changing multiple vindex columns",
"query": "update user_metadata set email = '[email protected]', address = '155 5th street' where user_id = 1",
"plan": {
"Type": "Passthrough",
"Type": "MultiShard",
"QueryType": "UPDATE",
"Original": "update user_metadata set email = '[email protected]', address = '155 5th street' where user_id = 1",
"Instructions": {
Expand Down Expand Up @@ -471,7 +471,7 @@
"comment": "update by primary keyspace id, changing one vindex column, using order by and limit",
"query": "update user_metadata set email = '[email protected]' where user_id = 1 order by user_id asc limit 10",
"plan": {
"Type": "Passthrough",
"Type": "MultiShard",
"QueryType": "UPDATE",
"Original": "update user_metadata set email = '[email protected]' where user_id = 1 order by user_id asc limit 10",
"Instructions": {
Expand Down Expand Up @@ -505,7 +505,7 @@
"comment": "update changes non owned vindex column",
"query": "update music_extra set music_id = 1 where user_id = 1",
"plan": {
"Type": "Passthrough",
"Type": "MultiShard",
"QueryType": "UPDATE",
"Original": "update music_extra set music_id = 1 where user_id = 1",
"Instructions": {
Expand Down Expand Up @@ -595,7 +595,7 @@
"comment": "delete from by primary keyspace id",
"query": "delete from user where id = 1",
"plan": {
"Type": "Passthrough",
"Type": "MultiShard",
"QueryType": "DELETE",
"Original": "delete from user where id = 1",
"Instructions": {
Expand Down Expand Up @@ -700,7 +700,7 @@
"comment": "routing rules: deleted from a routed table",
"query": "delete from route1 where id = 1",
"plan": {
"Type": "Passthrough",
"Type": "MultiShard",
"QueryType": "DELETE",
"Original": "delete from route1 where id = 1",
"Instructions": {
Expand Down Expand Up @@ -756,7 +756,7 @@
"comment": "update by lookup",
"query": "update music set val = 1 where id = 1",
"plan": {
"Type": "Passthrough",
"Type": "Lookup",
"QueryType": "UPDATE",
"Original": "update music set val = 1 where id = 1",
"Instructions": {
Expand Down Expand Up @@ -834,7 +834,7 @@
"comment": "delete from by lookup",
"query": "delete from music where id = 1",
"plan": {
"Type": "Passthrough",
"Type": "Lookup",
"QueryType": "DELETE",
"Original": "delete from music where id = 1",
"Instructions": {
Expand Down Expand Up @@ -2099,7 +2099,7 @@
"comment": "delete row in a multi column vindex table",
"query": "delete from multicolvin where kid=1",
"plan": {
"Type": "Passthrough",
"Type": "MultiShard",
"QueryType": "DELETE",
"Original": "delete from multicolvin where kid=1",
"Instructions": {
Expand Down Expand Up @@ -2130,7 +2130,7 @@
"comment": "update columns of multi column vindex",
"query": "update multicolvin set column_b = 1, column_c = 2 where kid = 1",
"plan": {
"Type": "Passthrough",
"Type": "MultiShard",
"QueryType": "UPDATE",
"Original": "update multicolvin set column_b = 1, column_c = 2 where kid = 1",
"Instructions": {
Expand Down Expand Up @@ -2164,7 +2164,7 @@
"comment": "update multiple vindexes, with multi column vindex",
"query": "update multicolvin set column_a = 0, column_b = 1, column_c = 2 where kid = 1",
"plan": {
"Type": "Passthrough",
"Type": "MultiShard",
"QueryType": "UPDATE",
"Original": "update multicolvin set column_a = 0, column_b = 1, column_c = 2 where kid = 1",
"Instructions": {
Expand Down Expand Up @@ -2694,7 +2694,7 @@
"comment": "update vindex value to null",
"query": "update user set name = null where id = 1",
"plan": {
"Type": "Passthrough",
"Type": "MultiShard",
"QueryType": "UPDATE",
"Original": "update user set name = null where id = 1",
"Instructions": {
Expand Down Expand Up @@ -2956,7 +2956,7 @@
"comment": "delete with single table targets",
"query": "delete music from music where id = 1",
"plan": {
"Type": "Passthrough",
"Type": "Lookup",
"QueryType": "DELETE",
"Original": "delete music from music where id = 1",
"Instructions": {
Expand Down Expand Up @@ -3038,7 +3038,7 @@
"comment": "update multi column vindex, without values for all the vindex columns",
"query": "update multicolvin set column_c = 2 where kid = 1",
"plan": {
"Type": "Passthrough",
"Type": "MultiShard",
"QueryType": "UPDATE",
"Original": "update multicolvin set column_c = 2 where kid = 1",
"Instructions": {
Expand Down Expand Up @@ -3072,7 +3072,7 @@
"comment": "update with binary value",
"query": "update user set name = _binary 'abc' where id = 1",
"plan": {
"Type": "Passthrough",
"Type": "MultiShard",
"QueryType": "UPDATE",
"Original": "update user set name = _binary 'abc' where id = 1",
"Instructions": {
Expand Down Expand Up @@ -3106,7 +3106,7 @@
"comment": "delete with binary value",
"query": "delete from user where name = _binary 'abc'",
"plan": {
"Type": "MultiShard",
"Type": "Lookup",
"QueryType": "DELETE",
"Original": "delete from user where name = _binary 'abc'",
"Instructions": {
Expand Down Expand Up @@ -3345,7 +3345,7 @@
"comment": "Delete on backfilling and non-backfilling unique lookup vindexes should be a delete equal",
"query": "delete from zlookup_unique.t1 where c2 = 10 and c3 = 20",
"plan": {
"Type": "Passthrough",
"Type": "Lookup",
"QueryType": "DELETE",
"Original": "delete from zlookup_unique.t1 where c2 = 10 and c3 = 20",
"Instructions": {
Expand Down Expand Up @@ -3376,7 +3376,7 @@
"comment": "Update on backfilling and non-backfilling unique lookup vindexes should be an equal",
"query": "update zlookup_unique.t1 set c2 = 1 where c2 = 10 and c3 = 20",
"plan": {
"Type": "Passthrough",
"Type": "Lookup",
"QueryType": "UPDATE",
"Original": "update zlookup_unique.t1 set c2 = 1 where c2 = 10 and c3 = 20",
"Instructions": {
Expand Down Expand Up @@ -3410,7 +3410,7 @@
"comment": "Delete EQUAL and IN on backfilling and non-backfilling unique lookup vindexes should be a delete IN",
"query": "delete from zlookup_unique.t1 where c2 = 10 and c3 in (20, 21)",
"plan": {
"Type": "MultiShard",
"Type": "Lookup",
"QueryType": "DELETE",
"Original": "delete from zlookup_unique.t1 where c2 = 10 and c3 in (20, 21)",
"Instructions": {
Expand Down Expand Up @@ -3441,7 +3441,7 @@
"comment": "Update EQUAL and IN on backfilling and non-backfilling unique lookup vindexes should be an update IN",
"query": "update zlookup_unique.t1 set c2 = 1 where c2 = 10 and c3 in (20, 21)",
"plan": {
"Type": "MultiShard",
"Type": "Lookup",
"QueryType": "UPDATE",
"Original": "update zlookup_unique.t1 set c2 = 1 where c2 = 10 and c3 in (20, 21)",
"Instructions": {
Expand Down Expand Up @@ -3648,7 +3648,7 @@
"comment": "delete with a multicol vindex",
"query": "delete from multicol_tbl where cola = 1 and colb = 2",
"plan": {
"Type": "Passthrough",
"Type": "MultiShard",
"QueryType": "DELETE",
"Original": "delete from multicol_tbl where cola = 1 and colb = 2",
"Instructions": {
Expand Down Expand Up @@ -3680,7 +3680,7 @@
"comment": "delete with a multicol vindex - reverse order",
"query": "delete from multicol_tbl where colb = 2 and cola = 1",
"plan": {
"Type": "Passthrough",
"Type": "MultiShard",
"QueryType": "DELETE",
"Original": "delete from multicol_tbl where colb = 2 and cola = 1",
"Instructions": {
Expand Down Expand Up @@ -3776,7 +3776,7 @@
"comment": "update with multicol and an owned vindex which changes",
"query": "update multicol_tbl set colc = 1 where cola = 1 and colb = 2",
"plan": {
"Type": "Passthrough",
"Type": "MultiShard",
"QueryType": "UPDATE",
"Original": "update multicol_tbl set colc = 1 where cola = 1 and colb = 2",
"Instructions": {
Expand Down Expand Up @@ -3811,7 +3811,7 @@
"comment": "update with routing using non-unique lookup vindex",
"query": "update multicol_tbl set x = 42 where name = 'foo'",
"plan": {
"Type": "MultiShard",
"Type": "Lookup",
"QueryType": "UPDATE",
"Original": "update multicol_tbl set x = 42 where name = 'foo'",
"Instructions": {
Expand Down Expand Up @@ -3935,7 +3935,7 @@
"comment": "update with routing using subsharding column with in query as lower cost over lookup vindex",
"query": "update multicol_tbl set x = 1 where name = 'foo' and cola = 2",
"plan": {
"Type": "MultiShard",
"Type": "Lookup",
"QueryType": "UPDATE",
"Original": "update multicol_tbl set x = 1 where name = 'foo' and cola = 2",
"Instructions": {
Expand Down Expand Up @@ -3963,7 +3963,7 @@
"comment": "delete with routing using non-unique lookup vindex",
"query": "delete from multicol_tbl where name = 'foo'",
"plan": {
"Type": "MultiShard",
"Type": "Lookup",
"QueryType": "DELETE",
"Original": "delete from multicol_tbl where name = 'foo'",
"Instructions": {
Expand Down Expand Up @@ -4056,7 +4056,7 @@
"comment": "delete with routing using subsharding column with in query as lower cost over lookup vindex",
"query": "delete from multicol_tbl where name = 'foo' and cola = 2",
"plan": {
"Type": "MultiShard",
"Type": "Lookup",
"QueryType": "DELETE",
"Original": "delete from multicol_tbl where name = 'foo' and cola = 2",
"Instructions": {
Expand Down Expand Up @@ -4916,7 +4916,7 @@
"comment": "update with routing using multi column vindex",
"query": "update user set col = 1 where (name, col) in (('aa', 'bb'), ('cc', 'dd'))",
"plan": {
"Type": "MultiShard",
"Type": "Lookup",
"QueryType": "UPDATE",
"Original": "update user set col = 1 where (name, col) in (('aa', 'bb'), ('cc', 'dd'))",
"Instructions": {
Expand Down Expand Up @@ -4944,7 +4944,7 @@
"comment": "delete with routing using multi column vindex",
"query": "delete from user where (name, col) in (('aa', 'bb'), ('cc', 'dd'))",
"plan": {
"Type": "MultiShard",
"Type": "Lookup",
"QueryType": "DELETE",
"Original": "delete from user where (name, col) in (('aa', 'bb'), ('cc', 'dd'))",
"Instructions": {
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/planbuilder/testdata/select_cases.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5832,7 +5832,7 @@
"comment": "Query with non-plannable lookup vindex",
"query": "SELECT * FROM user_metadata WHERE user_metadata.non_planable = 'foo'",
"plan": {
"Type": "MultiShard",
"Type": "Lookup",
"QueryType": "SELECT",
"Original": "SELECT * FROM user_metadata WHERE user_metadata.non_planable = 'foo'",
"Instructions": {
Expand Down

0 comments on commit 329f5be

Please sign in to comment.