Skip to content

Commit

Permalink
Adding support for: 1) data diff patch statements when there are sche…
Browse files Browse the repository at this point in the history 
…ma changes, and 2) charset/collation change patch statements.
  • Loading branch information
@fulghum
fulghum committed Jul 20, 2023
1 parent 3e8b35d commit 070127b
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 63 deletions.
15 changes: 12 additions & 3 deletions go/libraries/doltcore/diff/diffsplitter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,6 @@ func mapQuerySchemaToTargetSchema(query, target sql.Schema) (mapping []int, err
} else {
return nil, errors.New("expected column prefix of 'to_' or 'from_' (" + col.Name + ")")
}
if mapping[i] < 0 { // sanity check
return nil, errors.New("failed to map diff column: " + col.Name)
}
}
return
}
Expand Down Expand Up @@ -163,6 +160,10 @@ func (ds DiffSplitter) SplitDiffResultRow(row sql.Row) (from, to RowDiff, err er
from.RowDiff = Removed
for i := 0; i < ds.splitIdx; i++ {
j := ds.queryToTarget[i]
// skip any columns that aren't mapped
if j < 0 {
continue
}
from.Row[j] = row[i]
from.ColDiffs[j] = Removed
}
Expand All @@ -172,6 +173,10 @@ func (ds DiffSplitter) SplitDiffResultRow(row sql.Row) (from, to RowDiff, err er
to.RowDiff = Added
for i := ds.splitIdx; i < len(row); i++ {
j := ds.queryToTarget[i]
// skip any columns that aren't mapped
if j < 0 {
continue
}
to.Row[j] = row[i]
to.ColDiffs[j] = Added
}
Expand All @@ -181,6 +186,10 @@ func (ds DiffSplitter) SplitDiffResultRow(row sql.Row) (from, to RowDiff, err er
from.RowDiff = ModifiedOld
for i := 0; i < ds.splitIdx; i++ {
j := ds.queryToTarget[i]
// skip any columns that aren't mapped
if j < 0 {
continue
}
from.Row[j] = row[i]
}
to.Row = make(sql.Row, len(ds.targetSch))
Expand Down
18 changes: 8 additions & 10 deletions go/libraries/doltcore/sqle/dolt_patch_table_function.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -424,16 +424,7 @@ func canGetDataDiff(ctx *sql.Context, td diff.TableDelta) bool {
})
return false
}
// cannot sql diff
if td.ToSch == nil || (td.FromSch != nil && !schema.SchemasAreEqual(td.FromSch, td.ToSch)) {
// TODO(8/24/22 Zach): this is overly broad, we can absolutely do better
ctx.Session.Warn(&sql.Warning{
Level: "Warning",
Code: mysql.ERNotSupportedYet,
Message: fmt.Sprintf("Incompatible schema change, skipping data diff for table '%s'", td.ToName),
})
return false
}

return true
}

Expand Down Expand Up @@ -574,6 +565,13 @@ func GetNonCreateNonDropTableSqlSchemaDiff(td diff.TableDelta, toSchemas map[str
}
}

// Handle charset/collation changes
toCollation := toSch.GetCollation()
fromCollation := fromSch.GetCollation()
if toCollation != fromCollation {
ddlStatements = append(ddlStatements, sqlfmt.AlterTableCollateStmt(td.ToName, fromCollation, toCollation))
}

return ddlStatements, nil
}

Expand Down
166 changes: 116 additions & 50 deletions go/libraries/doltcore/sqle/enginetest/dolt_queries_diff.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3124,29 +3124,21 @@ var PatchTableFunctionScriptTests = []queries.ScriptTest{
SetUpScript: []string{
"set @Commit0 = HashOf('HEAD');",
"create table t (pk int primary key, c1 int, c2 int, c3 int, c4 int, c5 int comment 'tag:5');",
"call dolt_add('.')",
"insert into t values (0,1,2,3,4,5), (1,1,2,3,4,5);",
"set @Commit1 = '';",
"call dolt_commit_hash_out(@Commit1, '-am', 'inserting two rows into table t');",
"call dolt_commit('-Am', 'inserting two rows into table t');",
"alter table t rename column c1 to c0",
"alter table t drop column c4",
"alter table t add c6 bigint",
},
Assertions: []queries.ScriptTestAssertion{
{
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch(@Commit1, 'WORKING', 't')",
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch('HEAD', 'WORKING', 't')",
Expected: []sql.Row{
{1, "t", "schema", "ALTER TABLE `t` RENAME COLUMN `c1` TO `c0`;"},
{2, "t", "schema", "ALTER TABLE `t` DROP `c4`;"},
{3, "t", "schema", "ALTER TABLE `t` ADD `c6` bigint;"},
},
ExpectedWarning: 1235,
ExpectedWarningsCount: 1,
},
{
Query: "SHOW WARNINGS;",
Expected: []sql.Row{
{"Warning", 1235, "Incompatible schema change, skipping data diff for table 't'"},
{4, "t", "data", "UPDATE `t` SET `c0`=1 WHERE `pk`=0;"},
{5, "t", "data", "UPDATE `t` SET `c0`=1 WHERE `pk`=1;"},
},
},
{
Expand All @@ -3155,6 +3147,8 @@ var PatchTableFunctionScriptTests = []queries.ScriptTest{
{1, "STAGED", "WORKING", "t", "schema", "ALTER TABLE `t` RENAME COLUMN `c1` TO `c0`;"},
{2, "STAGED", "WORKING", "t", "schema", "ALTER TABLE `t` DROP `c4`;"},
{3, "STAGED", "WORKING", "t", "schema", "ALTER TABLE `t` ADD `c6` bigint;"},
{4, "STAGED", "WORKING", "t", "data", "UPDATE `t` SET `c0`=1 WHERE `pk`=0;"},
{5, "STAGED", "WORKING", "t", "data", "UPDATE `t` SET `c0`=1 WHERE `pk`=1;"},
},
},
{
Expand All @@ -3163,6 +3157,8 @@ var PatchTableFunctionScriptTests = []queries.ScriptTest{
{1, "STAGED", "WORKING", "t", "schema", "ALTER TABLE `t` RENAME COLUMN `c1` TO `c0`;"},
{2, "STAGED", "WORKING", "t", "schema", "ALTER TABLE `t` DROP `c4`;"},
{3, "STAGED", "WORKING", "t", "schema", "ALTER TABLE `t` ADD `c6` bigint;"},
{4, "STAGED", "WORKING", "t", "data", "UPDATE `t` SET `c0`=1 WHERE `pk`=0;"},
{5, "STAGED", "WORKING", "t", "data", "UPDATE `t` SET `c0`=1 WHERE `pk`=1;"},
},
},
{
Expand All @@ -3171,6 +3167,8 @@ var PatchTableFunctionScriptTests = []queries.ScriptTest{
{1, "WORKING", "STAGED", "t", "schema", "ALTER TABLE `t` RENAME COLUMN `c0` TO `c1`;"},
{2, "WORKING", "STAGED", "t", "schema", "ALTER TABLE `t` DROP `c6`;"},
{3, "WORKING", "STAGED", "t", "schema", "ALTER TABLE `t` ADD `c4` int;"},
{4, "WORKING", "STAGED", "t", "data", "UPDATE `t` SET `c1`=1,`c4`=4 WHERE `pk`=0;"},
{5, "WORKING", "STAGED", "t", "data", "UPDATE `t` SET `c1`=1,`c4`=4 WHERE `pk`=1;"},
},
},
{
Expand Down Expand Up @@ -3199,6 +3197,8 @@ var PatchTableFunctionScriptTests = []queries.ScriptTest{
{1, "t", "schema", "ALTER TABLE `t` RENAME COLUMN `c1` TO `c0`;"},
{2, "t", "schema", "ALTER TABLE `t` DROP `c4`;"},
{3, "t", "schema", "ALTER TABLE `t` ADD `c6` bigint;"},
{4, "t", "data", "UPDATE `t` SET `c0`=1 WHERE `pk`=0;"},
{5, "t", "data", "UPDATE `t` SET `c0`=1 WHERE `pk`=1;"},
},
},
},
Expand All @@ -3208,87 +3208,119 @@ var PatchTableFunctionScriptTests = []queries.ScriptTest{
SetUpScript: []string{
"create table t (pk int primary key, c1 varchar(20), c2 varchar(20));",
"call dolt_add('.')",
"set @Commit1 = '';",
"call dolt_commit_hash_out(@Commit1, '-am', 'creating table t');",
"call dolt_commit('-am', 'creating table t');",
"set @Commit1 = hashof('HEAD');",

"insert into t values(1, 'one', 'two');",
"set @Commit2 = '';",
"call dolt_commit_hash_out(@Commit2, '-am', 'inserting row 1 into t in main');",
"call dolt_commit('-am', 'inserting row 1 into t in main');",
"set @Commit2 = hashof('HEAD');",

"CALL DOLT_checkout('-b', 'branch1');",
"alter table t drop column c2;",
"set @Commit3 = '';",
"call dolt_commit_hash_out(@Commit3, '-am', 'dropping column c2 in branch1');",
"call dolt_commit('-am', 'dropping column c2 in branch1');",
"set @Commit3 = hashof('HEAD');",

"delete from t where pk=1;",
"set @Commit4 = '';",
"call dolt_commit_hash_out(@Commit4, '-am', 'deleting row 1 in branch1');",
"call dolt_commit('-am', 'deleting row 1 in branch1');",
"set @Commit4 = hashof('HEAD');",

"insert into t values (2, 'two');",
"set @Commit5 = '';",
"call dolt_commit_hash_out(@Commit5, '-am', 'inserting row 2 in branch1');",
"call dolt_commit('-am', 'inserting row 2 in branch1');",
"set @Commit5 = hashof('HEAD');",

"CALL DOLT_checkout('main');",
"insert into t values (2, 'two', 'three');",
"set @Commit6 = '';",
"call dolt_commit_hash_out(@Commit6, '-am', 'inserting row 2 in main');",
"call dolt_commit('-am', 'inserting row 2 in main');",
"set @Commit6 = hashof('HEAD');",

"create table newtable (pk int primary key);",
"insert into newtable values (1), (2);",
"set @Commit7 = '';",
"call dolt_commit_hash_out(@Commit7, '-Am', 'new table newtable');",
"call dolt_commit('-Am', 'new table newtable');",
"set @Commit7 = hashof('HEAD');",
},
Assertions: []queries.ScriptTestAssertion{
{
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch('main', 'branch1', 't');",
Expected: []sql.Row{{1, "t", "schema", "ALTER TABLE `t` DROP `c2`;"}},
},
{
Query: "SHOW WARNINGS",
Expected: []sql.Row{{"Warning", 1235, "Incompatible schema change, skipping data diff for table 't'"}},
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch('main', 'branch1', 't');",
Expected: []sql.Row{
{1, "t", "schema", "ALTER TABLE `t` DROP `c2`;"},
{2, "t", "data", "DELETE FROM `t` WHERE `pk`=1;"},
{3, "t", "data", "UPDATE `t` SET WHERE `pk`=2;"},
},
},
{
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch('main..branch1', 't');",
Expected: []sql.Row{{1, "t", "schema", "ALTER TABLE `t` DROP `c2`;"}},
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch('main..branch1', 't');",
Expected: []sql.Row{
{1, "t", "schema", "ALTER TABLE `t` DROP `c2`;"},
{2, "t", "data", "DELETE FROM `t` WHERE `pk`=1;"},
{3, "t", "data", "UPDATE `t` SET WHERE `pk`=2;"},
},
},
{
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch('main', 'branch1');",
Expected: []sql.Row{
{1, "newtable", "schema", "DROP TABLE `newtable`;"},
{2, "t", "schema", "ALTER TABLE `t` DROP `c2`;"},
{3, "t", "data", "DELETE FROM `t` WHERE `pk`=1;"},
{4, "t", "data", "UPDATE `t` SET WHERE `pk`=2;"},
},
},
{
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch('main..branch1');",
Expected: []sql.Row{
{1, "newtable", "schema", "DROP TABLE `newtable`;"},
{2, "t", "schema", "ALTER TABLE `t` DROP `c2`;"},
{3, "t", "data", "DELETE FROM `t` WHERE `pk`=1;"},
{4, "t", "data", "UPDATE `t` SET WHERE `pk`=2;"},
},
},
{
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch('branch1', 'main', 't');",
Expected: []sql.Row{{1, "t", "schema", "ALTER TABLE `t` ADD `c2` varchar(20);"}},
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch('branch1', 'main', 't');",
Expected: []sql.Row{
{1, "t", "schema", "ALTER TABLE `t` ADD `c2` varchar(20);"},
{2, "t", "data", "INSERT INTO `t` (`pk`,`c1`,`c2`) VALUES (1,'one','two');"},
{3, "t", "data", "UPDATE `t` SET `c2`='three' WHERE `pk`=2;"},
},
},
{
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch('branch1..main', 't');",
Expected: []sql.Row{{1, "t", "schema", "ALTER TABLE `t` ADD `c2` varchar(20);"}},
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch('branch1..main', 't');",
Expected: []sql.Row{
{1, "t", "schema", "ALTER TABLE `t` ADD `c2` varchar(20);"},
{2, "t", "data", "INSERT INTO `t` (`pk`,`c1`,`c2`) VALUES (1,'one','two');"},
{3, "t", "data", "UPDATE `t` SET `c2`='three' WHERE `pk`=2;"},
},
},
{
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch('main~2', 'branch1', 't');",
Expected: []sql.Row{{1, "t", "schema", "ALTER TABLE `t` DROP `c2`;"}},
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch('main~2', 'branch1', 't');",
Expected: []sql.Row{
{1, "t", "schema", "ALTER TABLE `t` DROP `c2`;"},
{2, "t", "data", "DELETE FROM `t` WHERE `pk`=1;"},
{3, "t", "data", "INSERT INTO `t` (`pk`,`c1`) VALUES (2,'two');"},
},
},
{
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch('main~2..branch1', 't');",
Expected: []sql.Row{{1, "t", "schema", "ALTER TABLE `t` DROP `c2`;"}},
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch('main~2..branch1', 't');",
Expected: []sql.Row{
{1, "t", "schema", "ALTER TABLE `t` DROP `c2`;"},
{2, "t", "data", "DELETE FROM `t` WHERE `pk`=1;"},
{3, "t", "data", "INSERT INTO `t` (`pk`,`c1`) VALUES (2,'two');"},
},
},
// Three dot
{
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch('main...branch1', 't');",
Expected: []sql.Row{{1, "t", "schema", "ALTER TABLE `t` DROP `c2`;"}},
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch('main...branch1', 't');",
Expected: []sql.Row{
{1, "t", "schema", "ALTER TABLE `t` DROP `c2`;"},
{2, "t", "data", "DELETE FROM `t` WHERE `pk`=1;"},
{3, "t", "data", "INSERT INTO `t` (`pk`,`c1`) VALUES (2,'two');"},
},
},
{
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch('main...branch1');",
Expected: []sql.Row{{1, "t", "schema", "ALTER TABLE `t` DROP `c2`;"}},
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch('main...branch1');",
Expected: []sql.Row{
{1, "t", "schema", "ALTER TABLE `t` DROP `c2`;"},
{2, "t", "data", "DELETE FROM `t` WHERE `pk`=1;"},
{3, "t", "data", "INSERT INTO `t` (`pk`,`c1`) VALUES (2,'two');"},
},
},
{
Query: "SELECT statement_order, table_name, diff_type, statement FROM dolt_patch('branch1...main', 't');",
Expand Down Expand Up @@ -3371,8 +3403,7 @@ var PatchTableFunctionScriptTests = []queries.ScriptTest{
SetUpScript: []string{
"CREATE TABLE parent (id int PRIMARY KEY, id_ext int, v1 int, v2 text COMMENT 'tag:1', INDEX v1 (v1));",
"CREATE TABLE child (id int primary key, v1 int);",
"call dolt_add('.')",
"call dolt_commit('-am', 'new tables')",
"call dolt_commit('-Am', 'new tables')",
"ALTER TABLE child ADD CONSTRAINT fk_named FOREIGN KEY (v1) REFERENCES parent(v1);",
"insert into parent values (0, 1, 2, NULL);",
"ALTER TABLE parent DROP PRIMARY KEY;",
Expand All @@ -3396,12 +3427,11 @@ var PatchTableFunctionScriptTests = []queries.ScriptTest{
{3, "STAGED", "parent", "schema", "ALTER TABLE `parent` DROP PRIMARY KEY;"},
{4, "STAGED", "parent", "schema", "ALTER TABLE `parent` ADD PRIMARY KEY (id,id_ext);"},
},
ExpectedWarningsCount: 2,
ExpectedWarningsCount: 1,
},
{
Query: "SHOW WARNINGS;",
Expected: []sql.Row{
{"Warning", 1235, "Incompatible schema change, skipping data diff for table 'child'"},
{"Warning", 1235, "Primary key sets differ between revisions for table 'parent', skipping data diff"},
},
},
Expand All @@ -3419,6 +3449,38 @@ var PatchTableFunctionScriptTests = []queries.ScriptTest{
},
},
},
{
Name: "charset and collation changes",
SetUpScript: []string{
"create table t (pk int primary key) collate='utf8mb4_0900_bin';",
"call dolt_commit('-Am', 'empty table')",
"set @commit0=hashof('HEAD');",
"insert into t values (1)",
"alter table t collate='utf8mb4_0900_ai_ci';",
"call dolt_commit('-am', 'inserting a row and altering the collation')",
"set @commit1=hashof('HEAD');",
"alter table t CHARACTER SET='utf8mb3';",
"insert into t values (2)",
"call dolt_commit('-am', 'inserting a row and altering the collation')",
"set @commit2=hashof('HEAD');",
},
Assertions: []queries.ScriptTestAssertion{
{
Query: "select * from dolt_patch(@commit1, @commit0);",
Expected: []sql.Row{
{1, doltCommit, doltCommit, "t", "schema", "ALTER TABLE `t` COLLATE='utf8mb4_0900_bin';"},
{2, doltCommit, doltCommit, "t", "data", "DELETE FROM `t` WHERE `pk`=1;"},
},
},
{
Query: "select * from dolt_patch(@commit1, @commit2);",
Expected: []sql.Row{
{1, doltCommit, doltCommit, "t", "schema", "ALTER TABLE `t` COLLATE='utf8mb3_general_ci';"},
{2, doltCommit, doltCommit, "t", "data", "INSERT INTO `t` (`pk`) VALUES (2);"},
},
},
},
},
{
Name: "patch DDL changes",
SetUpScript: []string{
Expand All @@ -3439,6 +3501,10 @@ var PatchTableFunctionScriptTests = []queries.ScriptTest{
Expected: []sql.Row{
{"ALTER TABLE `t` DROP `b`;"},
{"ALTER TABLE `t` ADD `d` int;"},
{"UPDATE `t` SET WHERE `pk`=1;"},
{"UPDATE `t` SET WHERE `pk`=2;"},
{"DELETE FROM `t` WHERE `pk`=3;"},
{"INSERT INTO `t` (`pk`,`a`,`c`,`d`) VALUES (7,7,7,7);"},
},
},
},
Expand Down
9 changes: 9 additions & 0 deletions go/libraries/doltcore/sqle/sqlfmt/schema_fmt.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,15 @@ func AlterTableDropIndexStmt(tableName string, idx schema.Index) string {
return b.String()
}

func AlterTableCollateStmt(tableName string, fromCollation, toCollation schema.Collation) string {
var b strings.Builder
b.WriteString("ALTER TABLE ")
b.WriteString(QuoteIdentifier(tableName))
toCollationId := sql.CollationID(toCollation)
b.WriteString(" COLLATE=" + QuoteComment(toCollationId.Name()) + ";")
return b.String()
}

func AlterTableAddForeignKeyStmt(fk doltdb.ForeignKey, sch, parentSch schema.Schema) string {
var b strings.Builder
b.WriteString("ALTER TABLE ")
Expand Down

0 comments on commit 070127b

Please sign in to comment.