Skip to content

Commit

Permalink
VReplication: Support both vindex col def formats when initing target…
Browse files Browse the repository at this point in the history 
… sequences (#16862)

Signed-off-by: Matt Lord <mattalord@gmail.com>
  • Loading branch information
@mattlord
mattlord authored Sep 30, 2024
1 parent 5484439 commit 2c78417
Show file tree
Hide file tree
Showing 2 changed files with 162 additions and 18 deletions.
34 changes: 23 additions & 11 deletions go/vt/vtctl/workflow/traffic_switcher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1431,7 +1431,7 @@ func (ts *trafficSwitcher) getTargetSequenceMetadata(ctx context.Context) (map[s
// The table name can be escaped in the vschema definition.
unescapedTableName, err := sqlescape.UnescapeID(tableName)
if err != nil {
return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid table name %s in keyspace %s: %v",
return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid table name %q in keyspace %s: %v",
tableName, keyspace, err)
}
select {
Expand Down Expand Up @@ -1480,7 +1480,7 @@ func (ts *trafficSwitcher) getTargetSequenceMetadata(ctx context.Context) (map[s
// The keyspace name could be escaped so we need to unescape it.
ks, err := sqlescape.UnescapeID(keyspace)
if err != nil { // Should never happen
return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid keyspace name %s: %v", keyspace, err)
return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid keyspace name %q: %v", keyspace, err)
}
searchGroup.Go(func() error {
return searchKeyspace(gctx, searchCompleted, ks)
Expand Down Expand Up @@ -1521,7 +1521,7 @@ func (ts *trafficSwitcher) findSequenceUsageInKeyspace(vschema *vschemapb.Keyspa
// in the vschema.
unescapedTable, err := sqlescape.UnescapeID(table)
if err != nil {
return nil, false, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid table name %s defined in the sequence table %+v: %v",
return nil, false, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid table name %q defined in the sequence table %+v: %v",
table, seqTable, err)
}
sm := &sequenceMetadata{
Expand All @@ -1533,17 +1533,17 @@ func (ts *trafficSwitcher) findSequenceUsageInKeyspace(vschema *vschemapb.Keyspa
if strings.Contains(seqTable.AutoIncrement.Sequence, ".") {
keyspace, tableName, found := strings.Cut(seqTable.AutoIncrement.Sequence, ".")
if !found {
return nil, false, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid sequence table name %s defined in the %s keyspace",
return nil, false, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid sequence table name %q defined in the %s keyspace",
seqTable.AutoIncrement.Sequence, ts.targetKeyspace)
}
// Unescape the table name and keyspace name as they may be escaped in the
// vschema definition if they e.g. contain dashes.
if keyspace, err = sqlescape.UnescapeID(keyspace); err != nil {
return nil, false, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid keyspace in qualified sequence table name %s defined in sequence table %+v: %v",
return nil, false, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid keyspace in qualified sequence table name %q defined in sequence table %+v: %v",
seqTable.AutoIncrement.Sequence, seqTable, err)
}
if tableName, err = sqlescape.UnescapeID(tableName); err != nil {
return nil, false, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid qualified sequence table name %s defined in sequence table %+v: %v",
return nil, false, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid qualified sequence table name %q defined in sequence table %+v: %v",
seqTable.AutoIncrement.Sequence, seqTable, err)
}
sm.backingTableKeyspace = keyspace
Expand All @@ -1557,24 +1557,36 @@ func (ts *trafficSwitcher) findSequenceUsageInKeyspace(vschema *vschemapb.Keyspa
} else {
sm.backingTableName, err = sqlescape.UnescapeID(seqTable.AutoIncrement.Sequence)
if err != nil {
return nil, false, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid sequence table name %s defined in sequence table %+v: %v",
return nil, false, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid sequence table name %q defined in sequence table %+v: %v",
seqTable.AutoIncrement.Sequence, seqTable, err)
}
seqTable.AutoIncrement.Sequence = sm.backingTableName
allFullyQualified = false
}
// The column names can be escaped in the vschema definition.
for i := range seqTable.ColumnVindexes {
unescapedColumn, err := sqlescape.UnescapeID(seqTable.ColumnVindexes[i].Column)
var (
unescapedColumn string
err error
)
if len(seqTable.ColumnVindexes[i].Columns) > 0 {
for n := range seqTable.ColumnVindexes[i].Columns {
unescapedColumn, err = sqlescape.UnescapeID(seqTable.ColumnVindexes[i].Columns[n])
seqTable.ColumnVindexes[i].Columns[n] = unescapedColumn
}
} else {
// This is the legacy vschema definition.
unescapedColumn, err = sqlescape.UnescapeID(seqTable.ColumnVindexes[i].Column)
seqTable.ColumnVindexes[i].Column = unescapedColumn
}
if err != nil {
return nil, false, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid sequence column vindex name %s defined in sequence table %+v: %v",
return nil, false, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid sequence column vindex name %q defined in sequence table %+v: %v",
seqTable.ColumnVindexes[i].Column, seqTable, err)
}
seqTable.ColumnVindexes[i].Column = unescapedColumn
}
unescapedAutoIncCol, err := sqlescape.UnescapeID(seqTable.AutoIncrement.Column)
if err != nil {
return nil, false, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid auto-increment column name %s defined in sequence table %+v: %v",
return nil, false, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid auto-increment column name %q defined in sequence table %+v: %v",
seqTable.AutoIncrement.Column, seqTable, err)
}
seqTable.AutoIncrement.Column = unescapedAutoIncCol
Expand Down
146 changes: 139 additions & 7 deletions go/vt/vtctl/workflow/traffic_switcher_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package workflow
import (
"context"
"fmt"
"reflect"
"strconv"
"strings"
"testing"
Expand Down Expand Up @@ -74,6 +73,7 @@ func TestGetTargetSequenceMetadata(t *testing.T) {
cell := "cell1"
workflow := "wf1"
table := "`t1`"
table2 := "t2"
unescapedTable := "t1"
sourceKeyspace := &testKeyspace{
KeyspaceName: "source-ks",
Expand Down Expand Up @@ -201,6 +201,138 @@ func TestGetTargetSequenceMetadata(t *testing.T) {
},
},
},
{
name: "sequences using vindexes with both column definition structures",
sourceVSchema: &vschema.Keyspace{
Vindexes: vindexes,
Tables: map[string]*vschema.Table{
"seq1": {
Type: "sequence",
},
"seq2": {
Type: "sequence",
},
},
},
targetVSchema: &vschema.Keyspace{
Vindexes: vindexes,
Tables: map[string]*vschema.Table{
table: {
ColumnVindexes: []*vschema.ColumnVindex{
{
Name: "xxhash",
Column: "col1",
},
},
AutoIncrement: &vschema.AutoIncrement{
Column: "col1",
Sequence: fmt.Sprintf("%s.seq1", sourceKeyspace.KeyspaceName),
},
},
table2: {
ColumnVindexes: []*vschema.ColumnVindex{
{
Name: "xxhash",
Columns: []string{"col2"},
},
},
AutoIncrement: &vschema.AutoIncrement{
Column: "col2",
Sequence: fmt.Sprintf("%s.seq2", sourceKeyspace.KeyspaceName),
},
},
},
},
want: map[string]*sequenceMetadata{
"seq1": {
backingTableName: "seq1",
backingTableKeyspace: "source-ks",
backingTableDBName: "vt_source-ks",
usingTableName: unescapedTable,
usingTableDBName: "vt_targetks",
usingTableDefinition: &vschema.Table{
ColumnVindexes: []*vschema.ColumnVindex{
{
Column: "col1",
Name: "xxhash",
},
},
AutoIncrement: &vschema.AutoIncrement{
Column: "col1",
Sequence: fmt.Sprintf("%s.seq1", sourceKeyspace.KeyspaceName),
},
},
},
"seq2": {
backingTableName: "seq2",
backingTableKeyspace: "source-ks",
backingTableDBName: "vt_source-ks",
usingTableName: table2,
usingTableDBName: "vt_targetks",
usingTableDefinition: &vschema.Table{
ColumnVindexes: []*vschema.ColumnVindex{
{
Columns: []string{"col2"},
Name: "xxhash",
},
},
AutoIncrement: &vschema.AutoIncrement{
Column: "col2",
Sequence: fmt.Sprintf("%s.seq2", sourceKeyspace.KeyspaceName),
},
},
},
},
},
{
name: "sequence with table having mult-col vindex",
sourceVSchema: &vschema.Keyspace{
Vindexes: vindexes,
Tables: map[string]*vschema.Table{
"seq1": {
Type: "sequence",
},
},
},
targetVSchema: &vschema.Keyspace{
Vindexes: vindexes,
Tables: map[string]*vschema.Table{
table: {
ColumnVindexes: []*vschema.ColumnVindex{
{
Name: "xxhash",
Columns: []string{"col3", "col4"},
},
},
AutoIncrement: &vschema.AutoIncrement{
Column: "col1",
Sequence: fmt.Sprintf("%s.seq1", sourceKeyspace.KeyspaceName),
},
},
},
},
want: map[string]*sequenceMetadata{
"seq1": {
backingTableName: "seq1",
backingTableKeyspace: "source-ks",
backingTableDBName: "vt_source-ks",
usingTableName: unescapedTable,
usingTableDBName: "vt_targetks",
usingTableDefinition: &vschema.Table{
ColumnVindexes: []*vschema.ColumnVindex{
{
Columns: []string{"col3", "col4"},
Name: "xxhash",
},
},
AutoIncrement: &vschema.AutoIncrement{
Column: "col1",
Sequence: fmt.Sprintf("%s.seq1", sourceKeyspace.KeyspaceName),
},
},
},
},
},
{
name: "invalid table name",
sourceVSchema: &vschema.Keyspace{
Expand Down Expand Up @@ -228,7 +360,7 @@ func TestGetTargetSequenceMetadata(t *testing.T) {
},
},
},
err: "invalid table name `my-`seq1` in keyspace source-ks: UnescapeID err: unexpected single backtick at position 3 in 'my-`seq1'",
err: "invalid table name \"`my-`seq1`\" in keyspace source-ks: UnescapeID err: unexpected single backtick at position 3 in 'my-`seq1'",
},
{
name: "invalid keyspace name",
Expand Down Expand Up @@ -257,7 +389,7 @@ func TestGetTargetSequenceMetadata(t *testing.T) {
},
},
},
err: "invalid keyspace in qualified sequence table name `ks`1`.`my-seq1` defined in sequence table column_vindexes:{column:\"`my-col`\" name:\"xxhash\"} auto_increment:{column:\"`my-col`\" sequence:\"`ks`1`.`my-seq1`\"}: UnescapeID err: unexpected single backtick at position 2 in 'ks`1'",
err: "invalid keyspace in qualified sequence table name \"`ks`1`.`my-seq1`\" defined in sequence table column_vindexes:{column:\"`my-col`\" name:\"xxhash\"} auto_increment:{column:\"`my-col`\" sequence:\"`ks`1`.`my-seq1`\"}: UnescapeID err: unexpected single backtick at position 2 in 'ks`1'",
},
{
name: "invalid auto-inc column name",
Expand Down Expand Up @@ -286,7 +418,7 @@ func TestGetTargetSequenceMetadata(t *testing.T) {
},
},
},
err: "invalid auto-increment column name `my`-col` defined in sequence table column_vindexes:{column:\"my-col\" name:\"xxhash\"} auto_increment:{column:\"`my`-col`\" sequence:\"my-seq1\"}: UnescapeID err: unexpected single backtick at position 2 in 'my`-col'",
err: "invalid auto-increment column name \"`my`-col`\" defined in sequence table column_vindexes:{column:\"my-col\" name:\"xxhash\"} auto_increment:{column:\"`my`-col`\" sequence:\"my-seq1\"}: UnescapeID err: unexpected single backtick at position 2 in 'my`-col'",
},
{
name: "invalid sequence name",
Expand Down Expand Up @@ -315,7 +447,7 @@ func TestGetTargetSequenceMetadata(t *testing.T) {
},
},
},
err: "invalid sequence table name `my-`seq1` defined in sequence table column_vindexes:{column:\"`my-col`\" name:\"xxhash\"} auto_increment:{column:\"`my-col`\" sequence:\"`my-`seq1`\"}: UnescapeID err: unexpected single backtick at position 3 in 'my-`seq1'",
err: "invalid sequence table name \"`my-`seq1`\" defined in sequence table column_vindexes:{column:\"`my-col`\" name:\"xxhash\"} auto_increment:{column:\"`my-col`\" sequence:\"`my-`seq1`\"}: UnescapeID err: unexpected single backtick at position 3 in 'my-`seq1'",
},
}

Expand Down Expand Up @@ -349,7 +481,7 @@ func TestGetTargetSequenceMetadata(t *testing.T) {
id: 1,
ws: env.ws,
workflow: workflow,
tables: []string{table},
tables: []string{table, table2},
sourceKeyspace: sourceKeyspace.KeyspaceName,
targetKeyspace: targetKeyspace.KeyspaceName,
sources: sources,
Expand All @@ -361,7 +493,7 @@ func TestGetTargetSequenceMetadata(t *testing.T) {
} else {
require.NoError(t, err)
}
require.True(t, reflect.DeepEqual(tc.want, got), "want: %v, got: %v", tc.want, got)
require.EqualValues(t, tc.want, got)
})
}
}
Expand Down

0 comments on commit 2c78417

Please sign in to comment.