From ad5d3ea979502d9fe4b57bf5330ac91ef20f0824 Mon Sep 17 00:00:00 2001 From: Helder Sousa Date: Sun, 5 Jan 2025 00:16:37 +0000 Subject: [PATCH] Improve logic to include originalUniqueKey in sharedUniqueKeys (#1453) Until now, when building the list of SharedUniqueKeys, the code was selecting only the originalUniqueKeys that existed in ghostUniqueKeys list. However, as part of the alter table, it is possible to add more unique keys or even composed primary keys with multiple columns. The new logic keeps the old behaviour (it matches originalUniqueKey with exactly the same ghostUniqueKey) but also supports the cases where the originalUniqueKey is now a subset of one of the new ghostUniqueKeys. If such a case happens, we can still use the originalUniqueKey as normal because a new ghostUniqueKey with the columns of originalUniqueKey plus some new columns is inherently unique just by the columns in the originalUniqueKey, no matter the value in the new columns of the new unique key. --- go/logic/inspect.go | 3 ++- go/logic/inspect_test.go | 4 +++- localtests/fail-no-shared-uk/create.sql | 4 ++-- localtests/fail-no-shared-uk/extra_args | 2 +- localtests/shared-uk/create.sql | 22 ++++++++++++++++++++++ localtests/shared-uk/extra_args | 1 + 6 files changed, 31 insertions(+), 5 deletions(-) create mode 100644 localtests/shared-uk/create.sql create mode 100644 localtests/shared-uk/extra_args diff --git a/go/logic/inspect.go b/go/logic/inspect.go index ea8c3adca..bfe2c1eb2 100644 --- a/go/logic/inspect.go +++ b/go/logic/inspect.go @@ -779,8 +779,9 @@ func (this *Inspector) getSharedUniqueKeys(originalUniqueKeys, ghostUniqueKeys [ // the ALTER is on the name itself... for _, originalUniqueKey := range originalUniqueKeys { for _, ghostUniqueKey := range ghostUniqueKeys { - if originalUniqueKey.Columns.EqualsByNames(&ghostUniqueKey.Columns) { + if originalUniqueKey.Columns.IsSubsetOf(&ghostUniqueKey.Columns) { uniqueKeys = append(uniqueKeys, originalUniqueKey) + break } } } diff --git a/go/logic/inspect_test.go b/go/logic/inspect_test.go index ca6ad8254..ac24f35ef 100644 --- a/go/logic/inspect_test.go +++ b/go/logic/inspect_test.go @@ -16,6 +16,7 @@ func TestInspectGetSharedUniqueKeys(t *testing.T) { origUniqKeys := []*sql.UniqueKey{ {Columns: *sql.NewColumnList([]string{"id", "item_id"})}, {Columns: *sql.NewColumnList([]string{"id", "org_id"})}, + {Columns: *sql.NewColumnList([]string{"id"})}, } ghostUniqKeys := []*sql.UniqueKey{ {Columns: *sql.NewColumnList([]string{"id", "item_id"})}, @@ -24,7 +25,8 @@ func TestInspectGetSharedUniqueKeys(t *testing.T) { } inspector := &Inspector{} sharedUniqKeys := inspector.getSharedUniqueKeys(origUniqKeys, ghostUniqKeys) - require.Len(t, sharedUniqKeys, 2) + require.Len(t, sharedUniqKeys, 3) require.Equal(t, "id,item_id", sharedUniqKeys[0].Columns.String()) require.Equal(t, "id,org_id", sharedUniqKeys[1].Columns.String()) + require.Equal(t, "id", sharedUniqKeys[2].Columns.String()) } diff --git a/localtests/fail-no-shared-uk/create.sql b/localtests/fail-no-shared-uk/create.sql index 5bb45f2a3..8f092e790 100644 --- a/localtests/fail-no-shared-uk/create.sql +++ b/localtests/fail-no-shared-uk/create.sql @@ -1,10 +1,10 @@ drop table if exists gh_ost_test; create table gh_ost_test ( - id int auto_increment, + id int not null, i int not null, ts timestamp, primary key(id) -) auto_increment=1; +); drop event if exists gh_ost_test; delimiter ;; diff --git a/localtests/fail-no-shared-uk/extra_args b/localtests/fail-no-shared-uk/extra_args index 379a77d85..6f66ccdb9 100644 --- a/localtests/fail-no-shared-uk/extra_args +++ b/localtests/fail-no-shared-uk/extra_args @@ -1 +1 @@ ---alter="drop primary key, add primary key (id, i)" +--alter="drop primary key, add primary key (i)" diff --git a/localtests/shared-uk/create.sql b/localtests/shared-uk/create.sql new file mode 100644 index 000000000..5bb45f2a3 --- /dev/null +++ b/localtests/shared-uk/create.sql @@ -0,0 +1,22 @@ +drop table if exists gh_ost_test; +create table gh_ost_test ( + id int auto_increment, + i int not null, + ts timestamp, + primary key(id) +) auto_increment=1; + +drop event if exists gh_ost_test; +delimiter ;; +create event gh_ost_test + on schedule every 1 second + starts current_timestamp + ends current_timestamp + interval 60 second + on completion not preserve + enable + do +begin + insert into gh_ost_test values (null, 11, now()); + insert into gh_ost_test values (null, 13, now()); + insert into gh_ost_test values (null, 17, now()); +end ;; diff --git a/localtests/shared-uk/extra_args b/localtests/shared-uk/extra_args new file mode 100644 index 000000000..379a77d85 --- /dev/null +++ b/localtests/shared-uk/extra_args @@ -0,0 +1 @@ +--alter="drop primary key, add primary key (id, i)"