From 30a50e3a1e81bbf63b19f15a553c0e8b0a79b68b Mon Sep 17 00:00:00 2001 From: zyguan Date: Tue, 7 May 2024 21:52:09 +0800 Subject: [PATCH 1/2] This is an automated cherry-pick of #53034 Signed-off-by: ti-chi-bot --- pkg/table/tables/BUILD.bazel | 4 + pkg/table/tables/index_test.go | 114 +++++++++++++++++++++ pkg/tablecodec/tablecodec.go | 4 + tests/integrationtest/r/table/index.result | 69 +++++++++++++ tests/integrationtest/t/table/index.test | 34 ++++++ 5 files changed, 225 insertions(+) diff --git a/pkg/table/tables/BUILD.bazel b/pkg/table/tables/BUILD.bazel index b10e03f4b7d4b..c8c4495690422 100644 --- a/pkg/table/tables/BUILD.bazel +++ b/pkg/table/tables/BUILD.bazel @@ -73,7 +73,11 @@ go_test( ], embed = [":tables"], flaky = True, +<<<<<<< HEAD shard_count = 30, +======= + shard_count = 32, +>>>>>>> 96f9797fca4 (tablecodec: fix the issue that decoding an index value might panic (#53034)) deps = [ "//pkg/ddl", "//pkg/ddl/util/callback", diff --git a/pkg/table/tables/index_test.go b/pkg/table/tables/index_test.go index 2cf4cee9ce5fe..9174ac33435c1 100644 --- a/pkg/table/tables/index_test.go +++ b/pkg/table/tables/index_test.go @@ -16,6 +16,7 @@ package tables_test import ( "context" + "strings" "testing" "github.com/pingcap/tidb/pkg/ddl" @@ -168,3 +169,116 @@ func buildTableInfo(t *testing.T, sql string) *model.TableInfo { require.NoError(t, err) return tblInfo } +<<<<<<< HEAD +======= + +func TestGenIndexValueFromIndex(t *testing.T) { + tblInfo := buildTableInfo(t, "create table a (a int primary key, b int not null, c text, unique key key_b(b));") + tblInfo.State = model.StatePublic + tbl, err := tables.TableFromMeta(lkv.NewPanickingAllocators(tblInfo.SepAutoInc(), 0), tblInfo) + require.NoError(t, err) + + sessionOpts := encode.SessionOptions{ + SQLMode: mysql.ModeStrictAllTables, + Timestamp: 1234567890, + } + + encoder, err := lkv.NewBaseKVEncoder(&encode.EncodingConfig{ + Table: tbl, + SessionOptions: sessionOpts, + }) + require.NoError(t, err) + encoder.SessionCtx.GetSessionVars().RowEncoder.Enable = true + + data1 := []types.Datum{ + types.NewIntDatum(1), + types.NewIntDatum(23), + types.NewStringDatum("4.csv"), + } + tctx := encoder.SessionCtx.GetTableCtx() + _, err = encoder.Table.AddRecord(tctx, data1) + require.NoError(t, err) + kvPairs := encoder.SessionCtx.TakeKvPairs() + + indexKey := kvPairs.Pairs[1].Key + indexValue := kvPairs.Pairs[1].Val + + _, idxID, _, err := tablecodec.DecodeIndexKey(indexKey) + require.NoError(t, err) + + idxInfo := model.FindIndexInfoByID(tbl.Meta().Indices, idxID) + + valueStr, err := tables.GenIndexValueFromIndex(indexKey, indexValue, tbl.Meta(), idxInfo) + require.NoError(t, err) + require.Equal(t, []string{"23"}, valueStr) +} + +func TestGenIndexValueWithLargePaddingSize(t *testing.T) { + // ref https://github.com/pingcap/tidb/issues/47115 + tblInfo := buildTableInfo(t, "create table t (a int, b int, k varchar(255), primary key (a, b), key (k))") + var idx table.Index + for _, idxInfo := range tblInfo.Indices { + if !idxInfo.Primary { + idx = tables.NewIndex(tblInfo.ID, tblInfo, idxInfo) + break + } + } + var a, b *model.ColumnInfo + for _, col := range tblInfo.Columns { + if col.Name.String() == "a" { + a = col + } else if col.Name.String() == "b" { + b = col + } + } + require.NotNil(t, a) + require.NotNil(t, b) + + store := testkit.CreateMockStore(t) + txn, err := store.Begin() + require.NoError(t, err) + mockCtx := mock.NewContext() + sc := mockCtx.GetSessionVars().StmtCtx + padding := strings.Repeat(" ", 128) + idxColVals := types.MakeDatums("abc" + padding) + handleColVals := types.MakeDatums(1, 2) + encodedHandle, err := codec.EncodeKey(sc.TimeZone(), nil, handleColVals...) + require.NoError(t, err) + commonHandle, err := kv.NewCommonHandle(encodedHandle) + require.NoError(t, err) + + key, _, err := idx.GenIndexKey(sc.ErrCtx(), sc.TimeZone(), idxColVals, commonHandle, nil) + require.NoError(t, err) + _, err = idx.Create(mockCtx.GetTableCtx(), txn, idxColVals, commonHandle, nil) + require.NoError(t, err) + val, err := txn.Get(context.Background(), key) + require.NoError(t, err) + colInfo := tables.BuildRowcodecColInfoForIndexColumns(idx.Meta(), tblInfo) + colInfo = append(colInfo, rowcodec.ColInfo{ + ID: a.ID, + IsPKHandle: false, + Ft: rowcodec.FieldTypeFromModelColumn(a), + }) + colInfo = append(colInfo, rowcodec.ColInfo{ + ID: b.ID, + IsPKHandle: false, + Ft: rowcodec.FieldTypeFromModelColumn(b), + }) + colVals, err := tablecodec.DecodeIndexKV(key, val, 1, tablecodec.HandleDefault, colInfo) + require.NoError(t, err) + require.Len(t, colVals, 3) + _, d, err := codec.DecodeOne(colVals[0]) + require.NoError(t, err) + require.Equal(t, "abc"+padding, d.GetString()) + _, d, err = codec.DecodeOne(colVals[1]) + require.NoError(t, err) + require.Equal(t, int64(1), d.GetInt64()) + _, d, err = codec.DecodeOne(colVals[2]) + require.NoError(t, err) + require.Equal(t, int64(2), d.GetInt64()) + handle, err := tablecodec.DecodeIndexHandle(key, val, 1) + require.NoError(t, err) + require.False(t, handle.IsInt()) + require.Equal(t, commonHandle.Encoded(), handle.Encoded()) +} +>>>>>>> 96f9797fca4 (tablecodec: fix the issue that decoding an index value might panic (#53034)) diff --git a/pkg/tablecodec/tablecodec.go b/pkg/tablecodec/tablecodec.go index d374c2edd851e..73130fcab28eb 100644 --- a/pkg/tablecodec/tablecodec.go +++ b/pkg/tablecodec/tablecodec.go @@ -877,7 +877,11 @@ func buildRestoredColumn(allCols []rowcodec.ColInfo) []rowcodec.ColInfo { } if collate.IsBinCollation(col.Ft.GetCollate()) { // Change the fieldType from string to uint since we store the number of the truncated spaces. + // NOTE: the corresponding datum is generated as `types.NewUintDatum(paddingSize)`, and the raw data is + // encoded via `encodeUint`. Thus we should mark the field type as unsigened here so that the BytesDecoder + // can decode it correctly later. Otherwise there might be issues like #47115. copyColInfo.Ft = types.NewFieldType(mysql.TypeLonglong) + copyColInfo.Ft.AddFlag(mysql.UnsignedFlag) } else { copyColInfo.Ft = allCols[i].Ft } diff --git a/tests/integrationtest/r/table/index.result b/tests/integrationtest/r/table/index.result index 8ee408a5bf243..e6cbccd36f934 100644 --- a/tests/integrationtest/r/table/index.result +++ b/tests/integrationtest/r/table/index.result @@ -16,3 +16,72 @@ insert into t values (4, 3, 3); Error 1062 (23000): Duplicate entry '3' for key 't.v2' set @@tidb_txn_assertion_level=default; set @@tidb_constraint_check_in_place=default; +<<<<<<< HEAD +======= +drop table if exists t; +create table t(a varchar(20), b varchar(20), unique index idx_a(a(1))); +insert into t values ('qaa', 'abc'); +insert into t values ('qbb', 'xyz'); +Error 1062 (23000): Duplicate entry 'q' for key 't.idx_a' +insert into t values ('rcc', 'xyz'); +select * from t order by a; +a b +qaa abc +rcc xyz +update t set a = 'qcc' where a = 'rcc'; +Error 1062 (23000): Duplicate entry 'q' for key 't.idx_a' +update ignore t set a = 'qcc' where a = 'rcc'; +Level Code Message +Warning 1062 Duplicate entry 'q' for key 't.idx_a' +drop table if exists t; +create table t (id int, a varchar(64), b varchar(64), c varchar(64), index idx_a(a(64))); +show create table t; +Table Create Table +t CREATE TABLE `t` ( + `id` int(11) DEFAULT NULL, + `a` varchar(64) DEFAULT NULL, + `b` varchar(64) DEFAULT NULL, + `c` varchar(64) DEFAULT NULL, + KEY `idx_a` (`a`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin +alter table t add index idx_b(b(64)); +show create table t; +Table Create Table +t CREATE TABLE `t` ( + `id` int(11) DEFAULT NULL, + `a` varchar(64) DEFAULT NULL, + `b` varchar(64) DEFAULT NULL, + `c` varchar(64) DEFAULT NULL, + KEY `idx_a` (`a`), + KEY `idx_b` (`b`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin +alter table t add index idx_c(c(32)); +show create table t; +Table Create Table +t CREATE TABLE `t` ( + `id` int(11) DEFAULT NULL, + `a` varchar(64) DEFAULT NULL, + `b` varchar(64) DEFAULT NULL, + `c` varchar(64) DEFAULT NULL, + KEY `idx_a` (`a`), + KEY `idx_b` (`b`), + KEY `idx_c` (`c`(32)) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin +alter table t modify column c varchar(32); +show create table t; +Table Create Table +t CREATE TABLE `t` ( + `id` int(11) DEFAULT NULL, + `a` varchar(64) DEFAULT NULL, + `b` varchar(64) DEFAULT NULL, + `c` varchar(32) DEFAULT NULL, + KEY `idx_a` (`a`), + KEY `idx_b` (`b`), + KEY `idx_c` (`c`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin +drop table t; +drop table if exists t; +create table t (a int, b int, k varchar(255), primary key (a, b), key k (k)); +insert into t values (1, 1, 'abc '); +drop table t; +>>>>>>> 96f9797fca4 (tablecodec: fix the issue that decoding an index value might panic (#53034)) diff --git a/tests/integrationtest/t/table/index.test b/tests/integrationtest/t/table/index.test index 529a32e94a949..d90bbe92ea476 100644 --- a/tests/integrationtest/t/table/index.test +++ b/tests/integrationtest/t/table/index.test @@ -20,3 +20,37 @@ insert into t values (4, 3, 3); set @@tidb_txn_assertion_level=default; set @@tidb_constraint_check_in_place=default; +<<<<<<< HEAD +======= +# TestDuplicateErrorOnPrefixIndex, Issue: #44316. +drop table if exists t; +create table t(a varchar(20), b varchar(20), unique index idx_a(a(1))); +insert into t values ('qaa', 'abc'); +-- error 1062 +insert into t values ('qbb', 'xyz'); +insert into t values ('rcc', 'xyz'); +select * from t order by a; +-- error 1062 +update t set a = 'qcc' where a = 'rcc'; +--enable_warnings; +update ignore t set a = 'qcc' where a = 'rcc'; +--disable_warnings; + +# Test Issue 48295. +drop table if exists t; +create table t (id int, a varchar(64), b varchar(64), c varchar(64), index idx_a(a(64))); +show create table t; +alter table t add index idx_b(b(64)); +show create table t; +alter table t add index idx_c(c(32)); +show create table t; +alter table t modify column c varchar(32); +show create table t; +drop table t; + +# Test Issue 47115. +drop table if exists t; +create table t (a int, b int, k varchar(255), primary key (a, b), key k (k)); +insert into t values (1, 1, 'abc '); +drop table t; +>>>>>>> 96f9797fca4 (tablecodec: fix the issue that decoding an index value might panic (#53034)) From f48af1d4b053e4a73032ceee31786a0ee93ef490 Mon Sep 17 00:00:00 2001 From: zyguan Date: Wed, 22 May 2024 09:05:03 +0000 Subject: [PATCH 2/2] resolve conflicts Signed-off-by: zyguan --- pkg/table/tables/BUILD.bazel | 6 +- pkg/table/tables/index_test.go | 50 +---------------- tests/integrationtest/r/table/index.result | 65 ---------------------- tests/integrationtest/t/table/index.test | 29 ---------- 4 files changed, 4 insertions(+), 146 deletions(-) diff --git a/pkg/table/tables/BUILD.bazel b/pkg/table/tables/BUILD.bazel index c8c4495690422..d14f8f7dbacb6 100644 --- a/pkg/table/tables/BUILD.bazel +++ b/pkg/table/tables/BUILD.bazel @@ -73,11 +73,7 @@ go_test( ], embed = [":tables"], flaky = True, -<<<<<<< HEAD - shard_count = 30, -======= - shard_count = 32, ->>>>>>> 96f9797fca4 (tablecodec: fix the issue that decoding an index value might panic (#53034)) + shard_count = 31, deps = [ "//pkg/ddl", "//pkg/ddl/util/callback", diff --git a/pkg/table/tables/index_test.go b/pkg/table/tables/index_test.go index 9174ac33435c1..93ab60f8b12f5 100644 --- a/pkg/table/tables/index_test.go +++ b/pkg/table/tables/index_test.go @@ -169,49 +169,6 @@ func buildTableInfo(t *testing.T, sql string) *model.TableInfo { require.NoError(t, err) return tblInfo } -<<<<<<< HEAD -======= - -func TestGenIndexValueFromIndex(t *testing.T) { - tblInfo := buildTableInfo(t, "create table a (a int primary key, b int not null, c text, unique key key_b(b));") - tblInfo.State = model.StatePublic - tbl, err := tables.TableFromMeta(lkv.NewPanickingAllocators(tblInfo.SepAutoInc(), 0), tblInfo) - require.NoError(t, err) - - sessionOpts := encode.SessionOptions{ - SQLMode: mysql.ModeStrictAllTables, - Timestamp: 1234567890, - } - - encoder, err := lkv.NewBaseKVEncoder(&encode.EncodingConfig{ - Table: tbl, - SessionOptions: sessionOpts, - }) - require.NoError(t, err) - encoder.SessionCtx.GetSessionVars().RowEncoder.Enable = true - - data1 := []types.Datum{ - types.NewIntDatum(1), - types.NewIntDatum(23), - types.NewStringDatum("4.csv"), - } - tctx := encoder.SessionCtx.GetTableCtx() - _, err = encoder.Table.AddRecord(tctx, data1) - require.NoError(t, err) - kvPairs := encoder.SessionCtx.TakeKvPairs() - - indexKey := kvPairs.Pairs[1].Key - indexValue := kvPairs.Pairs[1].Val - - _, idxID, _, err := tablecodec.DecodeIndexKey(indexKey) - require.NoError(t, err) - - idxInfo := model.FindIndexInfoByID(tbl.Meta().Indices, idxID) - - valueStr, err := tables.GenIndexValueFromIndex(indexKey, indexValue, tbl.Meta(), idxInfo) - require.NoError(t, err) - require.Equal(t, []string{"23"}, valueStr) -} func TestGenIndexValueWithLargePaddingSize(t *testing.T) { // ref https://github.com/pingcap/tidb/issues/47115 @@ -242,14 +199,14 @@ func TestGenIndexValueWithLargePaddingSize(t *testing.T) { padding := strings.Repeat(" ", 128) idxColVals := types.MakeDatums("abc" + padding) handleColVals := types.MakeDatums(1, 2) - encodedHandle, err := codec.EncodeKey(sc.TimeZone(), nil, handleColVals...) + encodedHandle, err := codec.EncodeKey(sc, nil, handleColVals...) require.NoError(t, err) commonHandle, err := kv.NewCommonHandle(encodedHandle) require.NoError(t, err) - key, _, err := idx.GenIndexKey(sc.ErrCtx(), sc.TimeZone(), idxColVals, commonHandle, nil) + key, _, err := idx.GenIndexKey(sc, idxColVals, commonHandle, nil) require.NoError(t, err) - _, err = idx.Create(mockCtx.GetTableCtx(), txn, idxColVals, commonHandle, nil) + _, err = idx.Create(mockCtx, txn, idxColVals, commonHandle, nil) require.NoError(t, err) val, err := txn.Get(context.Background(), key) require.NoError(t, err) @@ -281,4 +238,3 @@ func TestGenIndexValueWithLargePaddingSize(t *testing.T) { require.False(t, handle.IsInt()) require.Equal(t, commonHandle.Encoded(), handle.Encoded()) } ->>>>>>> 96f9797fca4 (tablecodec: fix the issue that decoding an index value might panic (#53034)) diff --git a/tests/integrationtest/r/table/index.result b/tests/integrationtest/r/table/index.result index e6cbccd36f934..291e81cfddecc 100644 --- a/tests/integrationtest/r/table/index.result +++ b/tests/integrationtest/r/table/index.result @@ -16,72 +16,7 @@ insert into t values (4, 3, 3); Error 1062 (23000): Duplicate entry '3' for key 't.v2' set @@tidb_txn_assertion_level=default; set @@tidb_constraint_check_in_place=default; -<<<<<<< HEAD -======= -drop table if exists t; -create table t(a varchar(20), b varchar(20), unique index idx_a(a(1))); -insert into t values ('qaa', 'abc'); -insert into t values ('qbb', 'xyz'); -Error 1062 (23000): Duplicate entry 'q' for key 't.idx_a' -insert into t values ('rcc', 'xyz'); -select * from t order by a; -a b -qaa abc -rcc xyz -update t set a = 'qcc' where a = 'rcc'; -Error 1062 (23000): Duplicate entry 'q' for key 't.idx_a' -update ignore t set a = 'qcc' where a = 'rcc'; -Level Code Message -Warning 1062 Duplicate entry 'q' for key 't.idx_a' -drop table if exists t; -create table t (id int, a varchar(64), b varchar(64), c varchar(64), index idx_a(a(64))); -show create table t; -Table Create Table -t CREATE TABLE `t` ( - `id` int(11) DEFAULT NULL, - `a` varchar(64) DEFAULT NULL, - `b` varchar(64) DEFAULT NULL, - `c` varchar(64) DEFAULT NULL, - KEY `idx_a` (`a`) -) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin -alter table t add index idx_b(b(64)); -show create table t; -Table Create Table -t CREATE TABLE `t` ( - `id` int(11) DEFAULT NULL, - `a` varchar(64) DEFAULT NULL, - `b` varchar(64) DEFAULT NULL, - `c` varchar(64) DEFAULT NULL, - KEY `idx_a` (`a`), - KEY `idx_b` (`b`) -) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin -alter table t add index idx_c(c(32)); -show create table t; -Table Create Table -t CREATE TABLE `t` ( - `id` int(11) DEFAULT NULL, - `a` varchar(64) DEFAULT NULL, - `b` varchar(64) DEFAULT NULL, - `c` varchar(64) DEFAULT NULL, - KEY `idx_a` (`a`), - KEY `idx_b` (`b`), - KEY `idx_c` (`c`(32)) -) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin -alter table t modify column c varchar(32); -show create table t; -Table Create Table -t CREATE TABLE `t` ( - `id` int(11) DEFAULT NULL, - `a` varchar(64) DEFAULT NULL, - `b` varchar(64) DEFAULT NULL, - `c` varchar(32) DEFAULT NULL, - KEY `idx_a` (`a`), - KEY `idx_b` (`b`), - KEY `idx_c` (`c`) -) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin -drop table t; drop table if exists t; create table t (a int, b int, k varchar(255), primary key (a, b), key k (k)); insert into t values (1, 1, 'abc '); drop table t; ->>>>>>> 96f9797fca4 (tablecodec: fix the issue that decoding an index value might panic (#53034)) diff --git a/tests/integrationtest/t/table/index.test b/tests/integrationtest/t/table/index.test index d90bbe92ea476..fa136e3922906 100644 --- a/tests/integrationtest/t/table/index.test +++ b/tests/integrationtest/t/table/index.test @@ -20,37 +20,8 @@ insert into t values (4, 3, 3); set @@tidb_txn_assertion_level=default; set @@tidb_constraint_check_in_place=default; -<<<<<<< HEAD -======= -# TestDuplicateErrorOnPrefixIndex, Issue: #44316. -drop table if exists t; -create table t(a varchar(20), b varchar(20), unique index idx_a(a(1))); -insert into t values ('qaa', 'abc'); --- error 1062 -insert into t values ('qbb', 'xyz'); -insert into t values ('rcc', 'xyz'); -select * from t order by a; --- error 1062 -update t set a = 'qcc' where a = 'rcc'; ---enable_warnings; -update ignore t set a = 'qcc' where a = 'rcc'; ---disable_warnings; - -# Test Issue 48295. -drop table if exists t; -create table t (id int, a varchar(64), b varchar(64), c varchar(64), index idx_a(a(64))); -show create table t; -alter table t add index idx_b(b(64)); -show create table t; -alter table t add index idx_c(c(32)); -show create table t; -alter table t modify column c varchar(32); -show create table t; -drop table t; - # Test Issue 47115. drop table if exists t; create table t (a int, b int, k varchar(255), primary key (a, b), key k (k)); insert into t values (1, 1, 'abc '); drop table t; ->>>>>>> 96f9797fca4 (tablecodec: fix the issue that decoding an index value might panic (#53034))