From ee00c60883f0c26bfa1e8dfdf21e499d85a80407 Mon Sep 17 00:00:00 2001 From: dongmen <414110582@qq.com> Date: Wed, 1 Nov 2023 14:59:51 +0800 Subject: [PATCH 1/2] fix BuildTiDBTableInfo function --- cdc/entry/mounter_test.go | 51 ++++++++++++++++++++++++++++++++++++++- cdc/model/sink.go | 16 +++++++++++- 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/cdc/entry/mounter_test.go b/cdc/entry/mounter_test.go index 7dc12279dcb..1a8aeac4759 100644 --- a/cdc/entry/mounter_test.go +++ b/cdc/entry/mounter_test.go @@ -986,11 +986,13 @@ func TestGetDefaultZeroValue(t *testing.T) { func TestBuildTableInfo(t *testing.T) { cases := []struct { + caseNumber int origin string recovered string recoveredWithNilCol string }{ { + 1, "CREATE TABLE t1 (c INT PRIMARY KEY)", "CREATE TABLE `BuildTiDBTableInfo` (\n" + " `c` int(0) NOT NULL,\n" + @@ -1002,6 +1004,7 @@ func TestBuildTableInfo(t *testing.T) { ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin", }, { + 2, "CREATE TABLE t1 (" + " c INT UNSIGNED," + " c2 VARCHAR(10) NOT NULL," + @@ -1023,6 +1026,7 @@ func TestBuildTableInfo(t *testing.T) { ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin", }, { + 3, "CREATE TABLE t1 (" + " c INT UNSIGNED," + " gen INT AS (c+1) VIRTUAL," + @@ -1048,6 +1052,7 @@ func TestBuildTableInfo(t *testing.T) { ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin", }, { + 4, "CREATE TABLE `t1` (" + " `a` int(11) NOT NULL," + " `b` int(11) DEFAULT NULL," + @@ -1069,6 +1074,39 @@ func TestBuildTableInfo(t *testing.T) { " PRIMARY KEY (`a`(0)) /*T![clustered_index] CLUSTERED */\n" + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin", }, + { + 5, + "CREATE TABLE your_table (" + + " id INT NOT NULL," + + " name VARCHAR(50) NOT NULL," + + " email VARCHAR(100) NOT NULL," + + " age INT NOT NULL ," + + " address VARCHAR(200) NOT NULL," + + " PRIMARY KEY (id, name)," + + " UNIQUE INDEX idx_unique_1 (id, email, age)," + + " UNIQUE INDEX idx_unique_2 (name, email, address)" + + " );", + "CREATE TABLE `BuildTiDBTableInfo` (\n" + + " `id` int(0) NOT NULL,\n" + + " `name` varchar(0) NOT NULL,\n" + + " `email` varchar(0) NOT NULL,\n" + + " `age` int(0) NOT NULL,\n" + + " `address` varchar(0) NOT NULL,\n" + + " PRIMARY KEY (`id`(0),`name`(0)) /*T![clustered_index] CLUSTERED */,\n" + + " UNIQUE KEY `idx_1` (`id`(0),`email`(0),`age`(0)),\n" + + " UNIQUE KEY `idx_2` (`name`(0),`email`(0),`address`(0))\n" + + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin", + "CREATE TABLE `BuildTiDBTableInfo` (\n" + + " `id` int(0) NOT NULL,\n" + + " `name` varchar(0) NOT NULL,\n" + + " `omitted` unspecified GENERATED ALWAYS AS (pass_generated_check) VIRTUAL,\n" + + " `omitted` unspecified GENERATED ALWAYS AS (pass_generated_check) VIRTUAL,\n" + + " `omitted` unspecified GENERATED ALWAYS AS (pass_generated_check) VIRTUAL,\n" + + " PRIMARY KEY (`id`(0),`name`(0)) /*T![clustered_index] CLUSTERED */,\n" + + " UNIQUE KEY `idx_1` (`id`(0),`omitted`(0),`omitted`(0)),\n" + + " UNIQUE KEY `idx_2` (`name`(0),`omitted`(0),`omitted`(0))\n" + + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin", + }, } p := parser.New() for _, c := range cases { @@ -1083,7 +1121,18 @@ func TestBuildTableInfo(t *testing.T) { handle := sqlmodel.GetWhereHandle(recoveredTI, recoveredTI) require.NotNil(t, handle.UniqueNotNullIdx) require.Equal(t, c.recovered, showCreateTable(t, recoveredTI)) - + // make sure BuildTiDBTableInfo indentify the correct primary key + if c.caseNumber == 5 { + inexes := recoveredTI.Indices + primaryCount := 0 + for i := range inexes { + if inexes[i].Primary { + primaryCount++ + } + } + require.Equal(t, 1, primaryCount) + require.Equal(t, 2, len(handle.UniqueNotNullIdx.Columns)) + } // mimic the columns are set to nil when old value feature is disabled for i := range cols { if !cols[i].Flag.IsHandleKey() { diff --git a/cdc/model/sink.go b/cdc/model/sink.go index 2e18678ee16..5d54eb28661 100644 --- a/cdc/model/sink.go +++ b/cdc/model/sink.go @@ -514,7 +514,10 @@ func BuildTiDBTableInfo(columns []*Column, indexColumns [][]int) *model.TableInf continue } if firstCol.Flag.IsPrimaryKey() { - indexInfo.Primary = true + // If all columns in the index are primary key, then the index is primary key. + if CheckIfIndexesPrimaryKey(colOffsets, columns) { + indexInfo.Primary = true + } indexInfo.Unique = true } if firstCol.Flag.IsUniqueKey() { @@ -538,6 +541,17 @@ func BuildTiDBTableInfo(columns []*Column, indexColumns [][]int) *model.TableInf return ret } +// CheckIfIndexesPrimaryKey checks if all columns in a index is primary key +func CheckIfIndexesPrimaryKey(columnOffsets []int, columns []*Column) bool { + for _, offset := range columnOffsets { + column := columns[offset] + if column == nil || !column.Flag.IsPrimaryKey() { + return false + } + } + return true +} + // ColumnValueString returns the string representation of the column value func ColumnValueString(c interface{}) string { var data string From 944876c069cb7583ff9742dbed3bf7e4f2c49a32 Mon Sep 17 00:00:00 2001 From: dongmen <414110582@qq.com> Date: Wed, 1 Nov 2023 15:15:35 +0800 Subject: [PATCH 2/2] resolve comments --- cdc/entry/mounter_test.go | 12 +++--------- cdc/model/sink.go | 26 +++++++++----------------- 2 files changed, 12 insertions(+), 26 deletions(-) diff --git a/cdc/entry/mounter_test.go b/cdc/entry/mounter_test.go index 1a8aeac4759..865a210a101 100644 --- a/cdc/entry/mounter_test.go +++ b/cdc/entry/mounter_test.go @@ -986,13 +986,11 @@ func TestGetDefaultZeroValue(t *testing.T) { func TestBuildTableInfo(t *testing.T) { cases := []struct { - caseNumber int origin string recovered string recoveredWithNilCol string }{ { - 1, "CREATE TABLE t1 (c INT PRIMARY KEY)", "CREATE TABLE `BuildTiDBTableInfo` (\n" + " `c` int(0) NOT NULL,\n" + @@ -1004,7 +1002,6 @@ func TestBuildTableInfo(t *testing.T) { ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin", }, { - 2, "CREATE TABLE t1 (" + " c INT UNSIGNED," + " c2 VARCHAR(10) NOT NULL," + @@ -1026,7 +1023,6 @@ func TestBuildTableInfo(t *testing.T) { ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin", }, { - 3, "CREATE TABLE t1 (" + " c INT UNSIGNED," + " gen INT AS (c+1) VIRTUAL," + @@ -1052,7 +1048,6 @@ func TestBuildTableInfo(t *testing.T) { ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin", }, { - 4, "CREATE TABLE `t1` (" + " `a` int(11) NOT NULL," + " `b` int(11) DEFAULT NULL," + @@ -1074,8 +1069,7 @@ func TestBuildTableInfo(t *testing.T) { " PRIMARY KEY (`a`(0)) /*T![clustered_index] CLUSTERED */\n" + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin", }, - { - 5, + { // This case is to check the primary key is correctly identified by BuildTiDBTableInfo "CREATE TABLE your_table (" + " id INT NOT NULL," + " name VARCHAR(50) NOT NULL," + @@ -1122,7 +1116,7 @@ func TestBuildTableInfo(t *testing.T) { require.NotNil(t, handle.UniqueNotNullIdx) require.Equal(t, c.recovered, showCreateTable(t, recoveredTI)) // make sure BuildTiDBTableInfo indentify the correct primary key - if c.caseNumber == 5 { + if i == 5 { inexes := recoveredTI.Indices primaryCount := 0 for i := range inexes { @@ -1174,7 +1168,7 @@ func TestNewDMRowChange(t *testing.T) { }, } p := parser.New() - for _, c := range cases { + for i, c := range cases { stmt, err := p.ParseOneStmt(c.origin, "", "") require.NoError(t, err) originTI, err := ddl.BuildTableInfoFromAST(stmt.(*ast.CreateTableStmt)) diff --git a/cdc/model/sink.go b/cdc/model/sink.go index 5d54eb28661..de182f243b0 100644 --- a/cdc/model/sink.go +++ b/cdc/model/sink.go @@ -514,23 +514,26 @@ func BuildTiDBTableInfo(columns []*Column, indexColumns [][]int) *model.TableInf continue } if firstCol.Flag.IsPrimaryKey() { - // If all columns in the index are primary key, then the index is primary key. - if CheckIfIndexesPrimaryKey(colOffsets, columns) { - indexInfo.Primary = true - } indexInfo.Unique = true } if firstCol.Flag.IsUniqueKey() { indexInfo.Unique = true } + isPrimary := true for _, offset := range colOffsets { - col := ret.Columns[offset] + col := columns[offset] + // When only all columns in the index are primary key, then the index is primary key. + if col == nil || !col.Flag.IsPrimaryKey() { + isPrimary = false + } + tiCol := ret.Columns[offset] indexCol := &model.IndexColumn{} - indexCol.Name = col.Name + indexCol.Name = tiCol.Name indexCol.Offset = offset indexInfo.Columns = append(indexInfo.Columns, indexCol) + indexInfo.Primary = isPrimary } // TODO: revert the "all column set index related flag" to "only the @@ -541,17 +544,6 @@ func BuildTiDBTableInfo(columns []*Column, indexColumns [][]int) *model.TableInf return ret } -// CheckIfIndexesPrimaryKey checks if all columns in a index is primary key -func CheckIfIndexesPrimaryKey(columnOffsets []int, columns []*Column) bool { - for _, offset := range columnOffsets { - column := columns[offset] - if column == nil || !column.Flag.IsPrimaryKey() { - return false - } - } - return true -} - // ColumnValueString returns the string representation of the column value func ColumnValueString(c interface{}) string { var data string