From b5066b28bb0a453cd431ad8269c81b78448d67f8 Mon Sep 17 00:00:00 2001 From: Rafael Chacon Date: Wed, 5 Aug 2020 16:18:25 -0400 Subject: [PATCH 1/2] Reverts delete by destination owned vindexes * Currently this functionality introduces a bug. Let's remove it for now while we implement a proper fix. Signed-off-by: Rafael Chacon --- go/vt/vtgate/engine/delete.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/go/vt/vtgate/engine/delete.go b/go/vt/vtgate/engine/delete.go index d1d935a6add..a04c6161d50 100644 --- a/go/vt/vtgate/engine/delete.go +++ b/go/vt/vtgate/engine/delete.go @@ -160,12 +160,10 @@ func (del *Delete) execDeleteByDestination(vcursor VCursor, bindVars map[string] BindVariables: bindVars, } } - if len(del.Table.Owned) > 0 { - err = del.deleteVindexEntries(vcursor, bindVars, rss) - if err != nil { - return nil, err - } - } + // TODO @rafael: Add supports for owned vindexes here. + // At the moment this will leave orphaned rows in the owned vindex. + // However when using this functionality we are assuming the user + // is intending to bypass V3 functionality. return execMultiShard(vcursor, rss, queries, del.MultiShardAutocommit) } From aa719a496dcf268dcf6bccca90b2db3007d849c2 Mon Sep 17 00:00:00 2001 From: Rafael Chacon Date: Wed, 5 Aug 2020 17:58:42 -0400 Subject: [PATCH 2/2] Keep deleteVindexEntries for scatter queries Signed-off-by: Rafael Chacon --- go/vt/vtgate/engine/delete.go | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/go/vt/vtgate/engine/delete.go b/go/vt/vtgate/engine/delete.go index a04c6161d50..7ab7b712635 100644 --- a/go/vt/vtgate/engine/delete.go +++ b/go/vt/vtgate/engine/delete.go @@ -83,7 +83,7 @@ func (del *Delete) Execute(vcursor VCursor, bindVars map[string]*querypb.BindVar case In: return del.execDeleteIn(vcursor, bindVars) case Scatter: - return del.execDeleteByDestination(vcursor, bindVars, key.DestinationAllShards{}) + return del.execDeleteScatter(vcursor, bindVars, key.DestinationAllShards{}) case ByDestination: return del.execDeleteByDestination(vcursor, bindVars, del.TargetDestination) default: @@ -167,6 +167,28 @@ func (del *Delete) execDeleteByDestination(vcursor VCursor, bindVars map[string] return execMultiShard(vcursor, rss, queries, del.MultiShardAutocommit) } +func (del *Delete) execDeleteScatter(vcursor VCursor, bindVars map[string]*querypb.BindVariable, dest key.Destination) (*sqltypes.Result, error) { + rss, _, err := vcursor.ResolveDestinations(del.Keyspace.Name, nil, []key.Destination{dest}) + if err != nil { + return nil, vterrors.Wrap(err, "execDeleteScatter") + } + + queries := make([]*querypb.BoundQuery, len(rss)) + for i := range rss { + queries[i] = &querypb.BoundQuery{ + Sql: del.Query, + BindVariables: bindVars, + } + } + if len(del.Table.Owned) > 0 { + err = del.deleteVindexEntries(vcursor, bindVars, rss) + if err != nil { + return nil, err + } + } + return execMultiShard(vcursor, rss, queries, del.MultiShardAutocommit) +} + // deleteVindexEntries performs an delete if table owns vindex. // Note: the commit order may be different from the DML order because it's possible // for DMLs to reuse existing transactions.