Skip to content

Commit

Permalink
SidecarDB: Don't ignore table charset and collation (#16670)
Browse files Browse the repository at this point in the history
Signed-off-by: Matt Lord <mattalord@gmail.com>
  • Loading branch information
mattlord committed Sep 8, 2024
1 parent 53dbede commit 0ccf2ce
Show file tree
Hide file tree
Showing 21 changed files with 106 additions and 20 deletions.
2 changes: 1 addition & 1 deletion go/vt/sidecardb/schema/misc/heartbeat.sql
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ CREATE TABLE IF NOT EXISTS heartbeat
tabletUid INT UNSIGNED NOT NULL,
ts BIGINT UNSIGNED NOT NULL,
PRIMARY KEY (`keyspaceShard`)
) engine = InnoDB
) ENGINE = InnoDB CHARSET = utf8mb4
2 changes: 1 addition & 1 deletion go/vt/sidecardb/schema/misc/reparent_journal.sql
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,4 @@ CREATE TABLE IF NOT EXISTS reparent_journal
`replication_position` varbinary(64000) DEFAULT NULL,

PRIMARY KEY (`time_created_ns`)
) ENGINE = InnoDB
) ENGINE = InnoDB CHARSET = utf8mb4
2 changes: 1 addition & 1 deletion go/vt/sidecardb/schema/onlineddl/schema_migrations.sql
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,4 @@ CREATE TABLE IF NOT EXISTS schema_migrations
KEY `table_complete_idx` (`migration_status`, `keyspace`(64), `mysql_table`(64), `completed_timestamp`),
KEY `migration_context_idx` (`migration_context`(64)),
KEY `reverted_uuid_idx` (`reverted_uuid`)
) ENGINE = InnoDB
) ENGINE = InnoDB CHARSET = utf8mb4
2 changes: 1 addition & 1 deletion go/vt/sidecardb/schema/schemaengine/tables.sql
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ CREATE TABLE IF NOT EXISTS tables
CREATE_STATEMENT longtext,
CREATE_TIME BIGINT,
PRIMARY KEY (TABLE_SCHEMA, TABLE_NAME)
) engine = InnoDB
) ENGINE = InnoDB CHARSET = utf8mb4
2 changes: 1 addition & 1 deletion go/vt/sidecardb/schema/schemaengine/udfs.sql
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ CREATE TABLE IF NOT EXISTS udfs
FUNCTION_RETURN_TYPE varchar(20) CHARACTER SET `utf8mb3` COLLATE `utf8mb3_bin` NOT NULL,
FUNCTION_TYPE varchar(20) CHARACTER SET `utf8mb3` COLLATE `utf8mb3_bin` NOT NULL,
PRIMARY KEY (FUNCTION_NAME)
) engine = InnoDB
) ENGINE = InnoDB CHARSET = utf8mb4
2 changes: 1 addition & 1 deletion go/vt/sidecardb/schema/schemaengine/views.sql
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ CREATE TABLE IF NOT EXISTS views
CREATE_STATEMENT longtext,
VIEW_DEFINITION longtext NOT NULL,
PRIMARY KEY (TABLE_SCHEMA, TABLE_NAME)
) engine = InnoDB
) ENGINE = InnoDB CHARSET = utf8mb4
2 changes: 1 addition & 1 deletion go/vt/sidecardb/schema/twopc/dt_participant.sql
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ CREATE TABLE IF NOT EXISTS dt_participant
keyspace varchar(256) NOT NULL,
shard varchar(256) NOT NULL,
primary key(dtid, id)
) ENGINE = InnoDB
) ENGINE = InnoDB CHARSET = utf8mb4
2 changes: 1 addition & 1 deletion go/vt/sidecardb/schema/twopc/dt_state.sql
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ CREATE TABLE IF NOT EXISTS dt_state
time_created bigint NOT NULL,
primary key(dtid),
key (time_created)
) ENGINE = InnoDB
) ENGINE = InnoDB CHARSET = utf8mb4
2 changes: 1 addition & 1 deletion go/vt/sidecardb/schema/twopc/redo_state.sql
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ CREATE TABLE IF NOT EXISTS redo_state(
state bigint NOT NULL,
time_created bigint NOT NULL,
primary key(dtid)
) ENGINE = InnoDB
) ENGINE = InnoDB CHARSET = utf8mb4
2 changes: 1 addition & 1 deletion go/vt/sidecardb/schema/twopc/redo_statement.sql
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ CREATE TABLE IF NOT EXISTS redo_statement(
id bigint NOT NULL,
statement mediumblob NOT NULL,
primary key(dtid, id)
) ENGINE = InnoDB
) ENGINE = InnoDB CHARSET = utf8mb4
2 changes: 1 addition & 1 deletion go/vt/sidecardb/schema/vdiff/vdiff.sql
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,4 @@ CREATE TABLE IF NOT EXISTS vdiff
UNIQUE KEY `uuid_idx` (`vdiff_uuid`),
KEY `state` (`state`),
KEY `ks_wf_idx` (`keyspace`(64), `workflow`(64))
) ENGINE = InnoDB
) ENGINE = InnoDB CHARSET = utf8mb4
2 changes: 1 addition & 1 deletion go/vt/sidecardb/schema/vdiff/vdiff_log.sql
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ CREATE TABLE IF NOT EXISTS vdiff_log
`created_at` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
`message` text NOT NULL,
PRIMARY KEY (`id`)
) ENGINE = InnoDB
) ENGINE = InnoDB CHARSET = utf8mb4
2 changes: 1 addition & 1 deletion go/vt/sidecardb/schema/vdiff/vdiff_table.sql
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,4 @@ CREATE TABLE IF NOT EXISTS vdiff_table
`created_at` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
`updated_at` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
PRIMARY KEY (`vdiff_id`, `table_name`)
) ENGINE = InnoDB
) ENGINE = InnoDB CHARSET = utf8mb4
2 changes: 1 addition & 1 deletion go/vt/sidecardb/schema/vreplication/copy_state.sql
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,4 @@ CREATE TABLE IF NOT EXISTS copy_state
`lastpk` varbinary(2000) DEFAULT NULL,
PRIMARY KEY (`id`),
KEY `vrepl_id` (`vrepl_id`,`table_name`)
) ENGINE = InnoDB
) ENGINE = InnoDB CHARSET = utf8mb4
2 changes: 1 addition & 1 deletion go/vt/sidecardb/schema/vreplication/post_copy_action.sql
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ CREATE TABLE IF NOT EXISTS post_copy_action(
action JSON NOT NULL,
UNIQUE KEY (vrepl_id, table_name),
PRIMARY KEY(id)
) ENGINE = InnoDB
) ENGINE = InnoDB CHARSET = utf8mb4
2 changes: 1 addition & 1 deletion go/vt/sidecardb/schema/vreplication/resharding_journal.sql
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ CREATE TABLE IF NOT EXISTS resharding_journal
`db_name` varbinary(255) DEFAULT NULL,
`val` blob,
PRIMARY KEY (`id`)
) ENGINE = InnoDB
) ENGINE = InnoDB CHARSET = utf8mb4
2 changes: 1 addition & 1 deletion go/vt/sidecardb/schema/vreplication/schema_version.sql
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,4 @@ CREATE TABLE IF NOT EXISTS schema_version
ddl BLOB DEFAULT NULL,
schemax LONGBLOB NOT NULL,
PRIMARY KEY (id)
) ENGINE = InnoDB
) ENGINE = InnoDB CHARSET = utf8mb4
2 changes: 1 addition & 1 deletion go/vt/sidecardb/schema/vreplication/vreplication.sql
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,4 @@ CREATE TABLE IF NOT EXISTS vreplication
`options` json NOT NULL,
PRIMARY KEY (`id`),
KEY `workflow_idx` (`workflow`(64))
) ENGINE = InnoDB
) ENGINE = InnoDB CHARSET = utf8mb4
2 changes: 1 addition & 1 deletion go/vt/sidecardb/schema/vreplication/vreplication_log.sql
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@ CREATE TABLE IF NOT EXISTS vreplication_log
`count` bigint NOT NULL DEFAULT '1',
PRIMARY KEY (`id`),
KEY `vrepl_id_idx` (`vrepl_id`)
) ENGINE = InnoDB
) ENGINE = InnoDB CHARSET = utf8mb4
2 changes: 1 addition & 1 deletion go/vt/sidecardb/sidecardb.go
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ func (si *schemaInit) getCurrentSchema(tableName string) (string, error) {
// or an ALTER if the table exists but has a different schema.
func (si *schemaInit) findTableSchemaDiff(tableName, current, desired string) (string, error) {
hints := &schemadiff.DiffHints{
TableCharsetCollateStrategy: schemadiff.TableCharsetCollateIgnoreAlways,
TableCharsetCollateStrategy: schemadiff.TableCharsetCollateIgnoreEmpty,
AlterTableAlgorithmStrategy: schemadiff.AlterTableAlgorithmStrategyCopy,
}
env := schemadiff.NewEnv(si.env, si.coll)
Expand Down
86 changes: 86 additions & 0 deletions go/vt/sidecardb/sidecardb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,3 +250,89 @@ func TestAlterTableAlgorithm(t *testing.T) {
})
}
}

// TestTableSchemaDiff ensures that the diff produced by schemaInit.findTableSchemaDiff
// is resulting in the expected alter table statements in a variety of scenarios.
func TestTableSchemaDiff(t *testing.T) {
si := &schemaInit{
env: vtenv.NewTestEnv(),
}

type testCase struct {
name string
table string
oldSchema string
newSchema string
expectNoDiff bool
expectedAlter string
}
testCases := []testCase{
{
name: "modify table charset",
table: "t1",
oldSchema: "create table if not exists _vt.t1(i int) charset=utf8mb4",
newSchema: "create table if not exists _vt.t(i int) charset=utf8mb3",
expectedAlter: "alter table _vt.t1 charset utf8mb3, algorithm = copy",
},
{
name: "empty charset",
table: "t1",
oldSchema: "create table if not exists _vt.t1(i int) charset=utf8mb4",
newSchema: "create table if not exists _vt.t(i int)",
expectNoDiff: true, // We're not specifying an explicit charset in the new schema, so we shouldn't see a diff.
},
{
name: "modify table engine",
table: "t1",
oldSchema: "create table if not exists _vt.t1(i int) engine=myisam",
newSchema: "create table if not exists _vt.t(i int) engine=innodb",
expectedAlter: "alter table _vt.t1 engine innodb, algorithm = copy",
},
{
name: "add, modify, transfer PK",
table: "t1",
oldSchema: "create table _vt.t1 (i int primary key, i1 varchar(10)) charset utf8mb4",
newSchema: "create table _vt.t1 (i int, i1 varchar(20) character set utf8mb3 collate utf8mb3_bin, i2 int, primary key (i2)) charset utf8mb4",
expectedAlter: "alter table _vt.t1 drop primary key, modify column i1 varchar(20) character set utf8mb3 collate utf8mb3_bin, add column i2 int, add primary key (i2), algorithm = copy",
},
{
name: "modify visibility and add comment",
table: "t1",
oldSchema: "create table if not exists _vt.t1(c1 int, c2 int, c3 varchar(100)) charset utf8mb4",
newSchema: "create table if not exists _vt.t1(c1 int, c2 int, c3 varchar(100) invisible comment 'hoping to drop') charset utf8mb4",
expectedAlter: "alter table _vt.t1 modify column c3 varchar(100) comment 'hoping to drop' invisible, algorithm = copy",
},
{
name: "add PK and remove index",
table: "t1",
oldSchema: "create table if not exists _vt.t1(c1 int, c2 int, c3 varchar(100), key (c2)) charset utf8mb4",
newSchema: "create table if not exists _vt.t1(c1 int primary key, c2 int, c3 varchar(100)) charset utf8mb4",
expectedAlter: "alter table _vt.t1 drop key c2, add primary key (c1), algorithm = copy",
},
{
name: "add generated col",
table: "t1",
oldSchema: "create table if not exists _vt.t1(c1 int primary key) charset utf8mb4",
newSchema: "create table if not exists _vt.t1(c1 int primary key, c2 varchar(10) generated always as ('hello')) charset utf8mb4",
expectedAlter: "alter table _vt.t1 add column c2 varchar(10) as ('hello') virtual, algorithm = copy",
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
diff, err := si.findTableSchemaDiff(tc.table, tc.oldSchema, tc.newSchema)
require.NoError(t, err)
if tc.expectNoDiff {
require.Empty(t, diff)
return
}
stmt, err := si.env.Parser().Parse(diff)
require.NoError(t, err)
alter, ok := stmt.(*sqlparser.AlterTable)
require.True(t, ok)
require.NotNil(t, alter)
t.Logf("alter: %s", sqlparser.String(alter))
require.Equal(t, strings.ToLower(tc.expectedAlter), strings.ToLower(sqlparser.String(alter)))
})
}
}

0 comments on commit 0ccf2ce

Please sign in to comment.