Merge pull request #8345 from planetscale/online-ddl-vrepl-suite-enum-pk

Online DDL/VReplication test suite: support ENUM as part of PRIMARY KEY

go/test/endtoend/onlineddl/vrepl_suite/onlineddl_vrepl_suite_test.go

@@ -279,10 +279,6 @@ func testSingle(t *testing.T, testName string) {
  selectBefore := fmt.Sprintf("select %s from %s %s", beforeColumns, beforeTableName, orderBy)
  selectAfter := fmt.Sprintf("select %s from %s %s", afterColumns, afterTableName, orderBy)
 
- // selectBeforeRS := mysqlExec(t, selectBefore, "")
- // selectAfterRS := mysqlExec(t, selectAfter, "")
- // require.Equal(t, selectBeforeRS.Rows, selectAfterRS.Rows, "results mismatch: (%s) amd (%s)", selectBefore, selectAfter)
-
  selectBeforeFile := createTempScript(t, selectBefore)
  defer os.Remove(selectBeforeFile)
  beforeOutput := mysqlClientExecFile(t, "", selectBeforeFile)
@@ -291,7 +287,7 @@ func testSingle(t *testing.T, testName string) {
  defer os.Remove(selectAfterFile)
  afterOutput := mysqlClientExecFile(t, "", selectAfterFile)
 
- require.Equal(t, beforeOutput, afterOutput, "results mismatch: (%s) amd (%s)", selectBefore, selectAfter)
+ require.Equal(t, beforeOutput, afterOutput, "results mismatch: (%s) and (%s)", selectBefore, selectAfter)
  }
 }
 

go/test/endtoend/onlineddl/vrepl_suite/untestdata/enum-pk/create.sql → go/test/endtoend/onlineddl/vrepl_suite/testdata/enum-pk/create.sql

@@ -2,10 +2,14 @@ drop table if exists onlineddl_test;
 create table onlineddl_test (
  id int auto_increment,
  i int not null,
- e enum('red', 'green', 'blue', 'orange') not null default 'red' collate 'utf8_bin',
+ e enum('red', 'green', 'blue', 'orange', 'yellow', 'grey', 'black') not null default 'red' collate 'utf8_bin',
  primary key(id, e)
 ) auto_increment=1;
 
+insert into onlineddl_test values (null, 2, 'yellow');
+insert into onlineddl_test values (null, 3, 'grey');
+insert into onlineddl_test values (11, 5, 'yellow');
+
 drop event if exists onlineddl_test;
 delimiter ;;
 create event onlineddl_test
@@ -16,6 +20,8 @@ create event onlineddl_test
  enable
  do
 begin
+ update onlineddl_test set e='black' where e='grey';
+ update onlineddl_test set e='black' where id=11;
  insert into onlineddl_test values (null, 11, 'red');
  set @last_insert_id := last_insert_id();
  insert into onlineddl_test values (@last_insert_id, 11, 'green');

go/test/endtoend/onlineddl/vrepl_suite/testdata/update-pk/create.sql

@@ -0,0 +1,33 @@
+drop table if exists onlineddl_test;
+create table onlineddl_test (
+ id int auto_increment,
+ i int not null,
+ color varchar(32) not null default '',
+ primary key(id, color)
+) auto_increment=1;
+
+insert into onlineddl_test values (null, 5, 'grey');
+insert into onlineddl_test values (null, 7, 'yellow');
+
+drop event if exists onlineddl_test;
+delimiter ;;
+create event onlineddl_test
+ on schedule every 1 second
+ starts current_timestamp
+ ends current_timestamp + interval 60 second
+ on completion not preserve
+ enable
+ do
+begin
+ update onlineddl_test set color='dark grey' where i = 5;
+ insert into onlineddl_test values (null, 11, 'red');
+ set @last_insert_id := last_insert_id();
+ insert into onlineddl_test values (@last_insert_id, 11, 'green');
+ insert into onlineddl_test values (null, 13, 'green');
+ insert into onlineddl_test values (null, 17, 'blue');
+ set @last_insert_id := last_insert_id();
+ update onlineddl_test set color='orange' where id = @last_insert_id;
+ insert into onlineddl_test values (null, 23, null);
+ set @last_insert_id := last_insert_id();
+ update onlineddl_test set i=i+1, color='black' where id = @last_insert_id;
+end ;;

go/vt/vttablet/onlineddl/vrepl.go

@@ -391,13 +391,24 @@ func (v *VRepl) analyzeTables(ctx context.Context, conn *dbconnpool.DBConnection
  return err
  }
 
+ for _, sourcePKColumn := range v.sharedPKColumns.Columns() {
+ mappedColumn := v.targetSharedColumns.GetColumn(sourcePKColumn.Name)
+ if sourcePKColumn.Type == vrepl.EnumColumnType && mappedColumn.Type == vrepl.EnumColumnType {
+ // An ENUM as part of PRIMARY KEY. We must convert it to text because OMG that's complicated.
+ // There's a scenario where a query may modify the enum value (and it's bad practice, seeing
+ // that it's part of the PK, but it's still valid), and in that case we must have the string value
+ // to be able to DELETE the old row
+ v.targetSharedColumns.SetEnumToTextConversion(mappedColumn.Name, sourcePKColumn.EnumValues)
+ v.enumToTextMap[sourcePKColumn.Name] = sourcePKColumn.EnumValues
+ }
+ }
+
  for i := range v.sourceSharedColumns.Columns() {
  sourceColumn := v.sourceSharedColumns.Columns()[i]
  mappedColumn := v.targetSharedColumns.Columns()[i]
  if sourceColumn.Type == vrepl.EnumColumnType && mappedColumn.Type != vrepl.EnumColumnType && mappedColumn.Charset != "" {
  // A column is converted from ENUM type to textual type
  v.targetSharedColumns.SetEnumToTextConversion(mappedColumn.Name, sourceColumn.EnumValues)
-
  v.enumToTextMap[sourceColumn.Name] = sourceColumn.EnumValues
  }
  }

go/vt/vttablet/tabletmanager/vreplication/replicator_plan.go

@@ -337,15 +337,18 @@ func (tp *TablePlan) applyChange(rowChange *binlogdatapb.RowChange, executor fun
  before = true
  vals := sqltypes.MakeRowTrusted(tp.Fields, rowChange.Before)
  for i, field := range tp.Fields {
- bindvars["b_"+field.Name] = sqltypes.ValueBindVariable(vals[i])
+ bindVar, err := tp.bindFieldVal(field, &vals[i])
+ if err != nil {
+ return nil, err
+ }
+ bindvars["b_"+field.Name] = bindVar
  }
  }
  if rowChange.After != nil {
  after = true
  vals := sqltypes.MakeRowTrusted(tp.Fields, rowChange.After)
  for i, field := range tp.Fields {
- val := &vals[i]
- bindVar, err := tp.bindFieldVal(field, val)
+ bindVar, err := tp.bindFieldVal(field, &vals[i])
  if err != nil {
  return nil, err
  }
@@ -372,6 +375,7 @@ func (tp *TablePlan) applyChange(rowChange *binlogdatapb.RowChange, executor fun
  if _, err := execParsedQuery(tp.Delete, bindvars, executor); err != nil {
  return nil, err
  }
+
  }
  return execParsedQuery(tp.Insert, bindvars, executor)
  }