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.

Sign up for GitHub

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

Jump to bottom

VReplication: Support both vindex col def formats when initing target sequences #16862

Merged
merged 3 commits into from
Sep 30, 2024
Merged

VReplication: Support both vindex col def formats when initing target sequences #16862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1f7d173
Support both vindex col def formats when initing target sequences
mattlord Sep 27, 2024
e536072
Add unit test
mattlord Sep 27, 2024
ab83107
Support multi-col vindexes
mattlord Sep 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 23 additions & 11 deletions go/vt/vtctl/workflow/traffic_switcher.go
View file
Open in desktop
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
Open in desktop
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
Loading