From 329f5be2285e9fb73ab54fd10d5fcb98405ca3b6 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Tue, 11 Feb 2025 14:42:44 +0530 Subject: [PATCH] update and delete having vindex changes be classified as multishard and using lookup vindex as lookup plan type Signed-off-by: Harshit Gangal --- go/vt/vtgate/engine/delete.go | 6 +- go/vt/vtgate/engine/plan.go | 41 +++++++++---- go/vt/vtgate/engine/update.go | 6 +- .../planbuilder/testdata/dml_cases.json | 58 +++++++++---------- .../planbuilder/testdata/select_cases.json | 2 +- 5 files changed, 69 insertions(+), 44 deletions(-) diff --git a/go/vt/vtgate/engine/delete.go b/go/vt/vtgate/engine/delete.go index 14859fc2135..f8425420974 100644 --- a/go/vt/vtgate/engine/delete.go +++ b/go/vt/vtgate/engine/delete.go @@ -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)) @@ -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, diff --git a/go/vt/vtgate/engine/plan.go b/go/vt/vtgate/engine/plan.go index bb2a5bd4c6c..5d7125f9049 100644 --- a/go/vt/vtgate/engine/plan.go +++ b/go/vt/vtgate/engine/plan.go @@ -59,8 +59,8 @@ const ( PlanLocal PlanPassthrough PlanMultiShard - PlanScatter PlanLookup + PlanScatter PlanJoinOp PlanForeignKey PlanComplex @@ -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), @@ -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 @@ -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 @@ -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 } diff --git a/go/vt/vtgate/engine/update.go b/go/vt/vtgate/engine/update.go index 49c8576fd84..6e153692089 100644 --- a/go/vt/vtgate/engine/update.go +++ b/go/vt/vtgate/engine/update.go @@ -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)) @@ -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, diff --git a/go/vt/vtgate/planbuilder/testdata/dml_cases.json b/go/vt/vtgate/planbuilder/testdata/dml_cases.json index 13252d68031..9b00821fb7b 100644 --- a/go/vt/vtgate/planbuilder/testdata/dml_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/dml_cases.json @@ -396,7 +396,7 @@ "comment": "update by primary keyspace id, changing one vindex column", "query": "update user_metadata set email = 'juan@vitess.io' where user_id = 1", "plan": { - "Type": "Passthrough", + "Type": "MultiShard", "QueryType": "UPDATE", "Original": "update user_metadata set email = 'juan@vitess.io' where user_id = 1", "Instructions": { @@ -436,7 +436,7 @@ "comment": "update by primary keyspace id, changing multiple vindex columns", "query": "update user_metadata set email = 'juan@vitess.io', address = '155 5th street' where user_id = 1", "plan": { - "Type": "Passthrough", + "Type": "MultiShard", "QueryType": "UPDATE", "Original": "update user_metadata set email = 'juan@vitess.io', address = '155 5th street' where user_id = 1", "Instructions": { @@ -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 = 'juan@vitess.io' where user_id = 1 order by user_id asc limit 10", "plan": { - "Type": "Passthrough", + "Type": "MultiShard", "QueryType": "UPDATE", "Original": "update user_metadata set email = 'juan@vitess.io' where user_id = 1 order by user_id asc limit 10", "Instructions": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.json b/go/vt/vtgate/planbuilder/testdata/select_cases.json index 5391d778c6f..bf6d358fb71 100644 --- a/go/vt/vtgate/planbuilder/testdata/select_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/select_cases.json @@ -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": {