From 635a65e608d6f24a4547c030bfa870d47eb95e4b Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Tue, 27 Jan 2026 22:35:30 +0800 Subject: [PATCH 01/14] Pick the test and default value filling behavior fix Signed-off-by: JaySon-Huang --- .../KVStore/Decode/RegionBlockReader.cpp | 3 + .../tests/gtest_region_block_reader.cpp | 85 ++++++++++++++++++- dbms/src/TiDB/Decode/RowCodec.cpp | 24 +++++- 3 files changed, 106 insertions(+), 6 deletions(-) diff --git a/dbms/src/Storages/KVStore/Decode/RegionBlockReader.cpp b/dbms/src/Storages/KVStore/Decode/RegionBlockReader.cpp index e31892dc9b9..02706d6ac17 100644 --- a/dbms/src/Storages/KVStore/Decode/RegionBlockReader.cpp +++ b/dbms/src/Storages/KVStore/Decode/RegionBlockReader.cpp @@ -179,6 +179,7 @@ bool RegionBlockReader::readImpl(Block & block, const ReadList & data_list, bool { VersionColResolver version_col_resolver; version_col_resolver.check(block, schema_snapshot->column_defines->size()); + // The column_ids to read according to schema_snapshot, each elem is (column_id, block_pos) const auto & read_column_ids = schema_snapshot->getColId2BlockPosMap(); const auto & pk_column_ids = schema_snapshot->pk_column_ids; const auto & pk_pos_map = schema_snapshot->pk_pos_map; @@ -269,6 +270,8 @@ bool RegionBlockReader::readImpl(Block & block, const ReadList & data_list, bool else { // Parse column value from encoded value + // Decode the column_ids from `column_ids_iter` to `read_column_ids.end()` + // and insert into `block` at position starting from `next_column_pos` if (!appendRowToBlock( *value_ptr, column_ids_iter, diff --git a/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp b/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp index 2c6f67f67ce..d1ab97eb1d4 100644 --- a/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp +++ b/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp @@ -354,7 +354,9 @@ TEST_F(RegionBlockReaderTest, MissingColumnRowV2) encodeColumns(table_info, fields, RowEncodeVersion::RowV2); auto new_table_info = getTableInfoWithMoreColumns({MutSup::extra_handle_id}, false); auto new_decoding_schema = getDecodingStorageSchemaSnapshot(new_table_info); - ASSERT_TRUE(decodeAndCheckColumns(new_decoding_schema, false)); + // `decodeAndCheckColumns` will return false because "column1" not_null=true but no_default_value=false + // FIXME: Re check the logic here + ASSERT_FALSE(decodeAndCheckColumns(new_decoding_schema, false)); } TEST_F(RegionBlockReaderTest, MissingColumnRowV1) @@ -363,7 +365,9 @@ TEST_F(RegionBlockReaderTest, MissingColumnRowV1) encodeColumns(table_info, fields, RowEncodeVersion::RowV1); auto new_table_info = getTableInfoWithMoreColumns({MutSup::extra_handle_id}, false); auto new_decoding_schema = getDecodingStorageSchemaSnapshot(new_table_info); - ASSERT_TRUE(decodeAndCheckColumns(new_decoding_schema, false)); + // `decodeAndCheckColumns` will return false because "column1" not_null=true but no_default_value=false + // FIXME: Re check the logic here + ASSERT_FALSE(decodeAndCheckColumns(new_decoding_schema, false)); } TEST_F(RegionBlockReaderTest, ExtraColumnRowV2) @@ -692,5 +696,82 @@ try } CATCH +TEST_F(RegionBlockReaderTest, ReadFromRegionDefaultValue) +try +{ + // With this table_info, c1 is filled with "0" according to ori_default + TableInfo table_info( + R"({"cols":[{"id":1,"name":{"L":"c0","O":"c0"},"offset":0,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":4,"Tp":1}},{"id":2,"name":{"L":"handle","O":"handle"},"offset":1,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":515,"Flen":11,"Tp":3}},{"default":"-56083770","id":7,"name":{"L":"c1","O":"c1"},"offset":2,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":1,"Flen":20,"Tp":8}},{"id":4,"name":{"L":"c2","O":"c2"},"offset":3,"origin_default":"0.07954397","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":-1,"Flag":4097,"Flen":12,"Tp":4}},{"id":5,"name":{"L":"c5","O":"c5"},"offset":4,"origin_default":"0","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}},{"default":"247262911","id":6,"name":{"L":"c4","O":"c4"},"offset":5,"origin_default":"247262911","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}}],"id":711,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"t0","O":"t0"},"pk_is_handle":true,"state":5,"tiflash_replica":{"Available":true,"Count":1},"update_timestamp":463845180343844895})", + NullspaceID); + + RegionID region_id = 4; + // the start_key and end_key for table_id = 68 + String region_start_key(bytesFromHexString("7480000000000002FF7C5F720000000000FA")); + String region_end_key(bytesFromHexString("7480000000000002FF7D00000000000000F8")); + auto region = RegionBench::makeRegionForRange(region_id, region_start_key, region_end_key); + // the hex kv dump from SSTFile + std::vector> kvs = { + // { + // "7480000000000002FFA95F728000000000FF0000010000000000FAF9901806DEF7FFDA", + // "50A380A08892FFF9B706762C8000040000000405060708000F0016001A00BFDC4011A00000000A0080000000000A008000003CA339" + // "1ABC85", + // }, + // { + // "7480000000000002FFA95F728000000000FF0000010000000000FAF9901806DEF7FFD8", + // "50A680A08892FFF9B706762C8000040000000405060708000F0016001A00BFDC4011A00000000A008033E04D600A008000003CA339" + // "1ABC85", + // }, + { + "7480000000000002FFA95F728000000000FF0000020000000000FAF9901806DE33FFE8", + "509680B08E92FFF9B706762580000300000004050608000F001600BF720CDD400000000A0080000000010A00800000393C", + }, + }; + for (const auto & [k, v] : kvs) + { + region->insertDebug("write", TiKVKey(bytesFromHexString(k)), TiKVValue(bytesFromHexString(v))); + } + + auto data_list_read = ReadRegionCommitCache(region, true); + ASSERT_TRUE(data_list_read.has_value()); + + auto decoding_schema = getDecodingStorageSchemaSnapshot(table_info); + { + // force_decode=false can not decode because there are + // missing value for column with not null flag. + auto reader = RegionBlockReader(decoding_schema); + Block res_block = createBlockSortByColumnID(decoding_schema); + ASSERT_FALSE(reader.read(res_block, *data_list_read, false)); + } + { + // force_decode=true can decode the block, and filling the default value for c1 + auto reader = RegionBlockReader(decoding_schema); + Block res_block = createBlockSortByColumnID(decoding_schema); + ASSERT_TRUE(reader.read(res_block, *data_list_read, true)); + // TODO: verify the default value is filled correctly + LOG_INFO( + Logger::get(), + "Decoded block:\n{}", + DB::tests::getColumnsContent(res_block.getColumnsWithTypeAndName())); + } + + // With this table_info, c1 does not have the "not null" flag + TableInfo table_info_c1_nullable( + R"({"cols":[{"id":1,"name":{"L":"c0","O":"c0"},"offset":0,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":4,"Tp":1}},{"id":2,"name":{"L":"handle","O":"handle"},"offset":1,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":515,"Flen":11,"Tp":3}},{"id":7,"name":{"L":"c1","O":"c1"},"offset":2,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":20,"Tp":8}},{"id":4,"name":{"L":"c2","O":"c2"},"offset":3,"origin_default":"0.07954397","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":-1,"Flag":4097,"Flen":12,"Tp":4}},{"id":5,"name":{"L":"c5","O":"c5"},"offset":4,"origin_default":"0","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}},{"default":"247262911","id":6,"name":{"L":"c4","O":"c4"},"offset":5,"origin_default":"247262911","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}}],"id":681,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"t0","O":"t0"},"pk_is_handle":true,"state":5,"tiflash_replica":{"Available":true,"Count":1},"update_timestamp":463844343842340870})", + NullspaceID); + + decoding_schema = getDecodingStorageSchemaSnapshot(table_info_c1_nullable); + { + // force_decode=false should be able to decode because c1 is nullable + auto reader = RegionBlockReader(decoding_schema); + Block res_block = createBlockSortByColumnID(decoding_schema); + ASSERT_TRUE(reader.read(res_block, *data_list_read, false)); + // TODO: verify the default value is filled correctly + LOG_INFO( + Logger::get(), + "Decoded block:\n{}", + DB::tests::getColumnsContent(res_block.getColumnsWithTypeAndName())); + } +} +CATCH } // namespace DB::tests diff --git a/dbms/src/TiDB/Decode/RowCodec.cpp b/dbms/src/TiDB/Decode/RowCodec.cpp index 79ffc2dba99..ad5bb879438 100644 --- a/dbms/src/TiDB/Decode/RowCodec.cpp +++ b/dbms/src/TiDB/Decode/RowCodec.cpp @@ -412,10 +412,15 @@ inline bool addDefaultValueToColumnIfPossible( // fallthrough to fill default value when force_decode } - if (column_info.hasNoDefaultValueFlag() && column_info.hasNotNullFlag()) + if (column_info.hasNotNullFlag()) { if (!force_decode) + { + // This is a Column that does not have encoded datum in the value, but it is defined as NOT NULL. + // It could be a row encoded by newer schema after turning `NOT NULL` to `NULLABLE`. + // Return false to trigger schema sync when `force_decode==false`. return false; + } // Else the row does not contain this "not null" / "no default value" column, // it could be a row encoded by older schema. // fallthrough to fill default value when force_decode @@ -460,7 +465,9 @@ bool appendRowV2ToBlockImpl( num_not_null_columns, value_offsets); size_t values_start_pos = cursor; + // how many not null columns have been processed size_t idx_not_null = 0; + // how many null columns have been processed size_t idx_null = 0; // Merge ordered not null/null columns to keep order. while (idx_not_null < not_null_column_ids.size() || idx_null < null_column_ids.size()) @@ -481,12 +488,13 @@ bool appendRowV2ToBlockImpl( const auto next_column_id = column_ids_iter->first; if (next_column_id > next_datum_column_id) { - // The next column id to read is bigger than the column id of next datum in encoded row. + // The next_column_id to read is bigger than the next_datum_column_id in encoded row. // It means this is the datum of extra column. May happen when reading after dropping // a column. + // For `force_decode == false`, we should return false to let upper layer trigger schema sync. if (!force_decode) return false; - // Ignore the extra column and continue to parse other datum + // For `force_decode == true`, we just skip this extra column and continue to parse other datum. if (is_null) idx_null++; else @@ -494,7 +502,7 @@ bool appendRowV2ToBlockImpl( } else if (next_column_id < next_datum_column_id) { - // The next column id to read is less than the column id of next datum in encoded row. + // The next_column_id to read is less than the next_datum_column_id in encoded row. // It means this is the datum of missing column. May happen when reading after adding // a column. // Fill with default value and continue to read data for next column id. @@ -505,7 +513,9 @@ bool appendRowV2ToBlockImpl( block_column_pos, ignore_pk_if_absent, force_decode)) + { return false; + } column_ids_iter++; block_column_pos++; } @@ -570,8 +580,12 @@ bool appendRowV2ToBlockImpl( block_column_pos++; } } + + // There are more columns to read other than the datum encoded in the row. while (column_ids_iter != column_ids_iter_end) { + // Skip if the column_id is the same as `pk_handle_id`. The value of column + // `pk_handle_id` will be filled in upper layer but not in this function. if (column_ids_iter->first != pk_handle_id) { const auto & column_info = column_infos[column_ids_iter->second]; @@ -581,7 +595,9 @@ bool appendRowV2ToBlockImpl( block_column_pos, ignore_pk_if_absent, force_decode)) + { return false; + } } column_ids_iter++; block_column_pos++; From c71f617d0ce4fccf7167f1463b6e0f243feec5b0 Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Tue, 27 Jan 2026 23:38:12 +0800 Subject: [PATCH 02/14] Refine the logic for filling in default values with finer granularity Signed-off-by: JaySon-Huang --- dbms/src/TiDB/Decode/RowCodec.cpp | 47 ++++++++++++++++++++++--------- dbms/src/TiDB/Schema/TiDB.cpp | 5 ++++ dbms/src/TiDB/Schema/TiDB.h | 1 + 3 files changed, 40 insertions(+), 13 deletions(-) diff --git a/dbms/src/TiDB/Decode/RowCodec.cpp b/dbms/src/TiDB/Decode/RowCodec.cpp index ad5bb879438..e5ba6aa9a61 100644 --- a/dbms/src/TiDB/Decode/RowCodec.cpp +++ b/dbms/src/TiDB/Decode/RowCodec.cpp @@ -388,6 +388,8 @@ bool appendRowToBlock( } } +// When the `column_info` is missing in the encoded row, we try to add default value to the `block` at `block_column_pos`. +// Return true if we could add default value to the column. Otherwise false and the caller should trigger schema sync. inline bool addDefaultValueToColumnIfPossible( const ColumnInfo & column_info, Block & block, @@ -395,12 +397,10 @@ inline bool addDefaultValueToColumnIfPossible( bool ignore_pk_if_absent, bool force_decode) { - // We consider a missing column could be safely filled with NULL, unless it has not default value and is NOT NULL. - // This could saves lots of unnecessary schema syncs for old data with a newer schema that has newly added columns. - if (column_info.hasPriKeyFlag()) { - // For clustered index or pk_is_handle, if the pk column does not exists, it can still be decoded from the key + // For clustered index or pk_is_handle, if the pk column does not exists, it can still be decoded from the key. + // just skip this column. if (ignore_pk_if_absent) return true; @@ -409,23 +409,44 @@ inline bool addDefaultValueToColumnIfPossible( return false; // Else non-clustered index, and not pk_is_handle, it could be a row encoded by older schema, // we need to fill the column which has primary key flag with default value. - // fallthrough to fill default value when force_decode + // fallthrough to fill default value when `force_decode==true` } if (column_info.hasNotNullFlag()) { if (!force_decode) { - // This is a Column that does not have encoded datum in the value, but it is defined as NOT NULL. - // It could be a row encoded by newer schema after turning `NOT NULL` to `NULLABLE`. - // Return false to trigger schema sync when `force_decode==false`. - return false; + if (column_info.hasNoDefaultValueFlag()) + { + // This is a Column that defined as NOT NULL but no default value. In this case, user + // should fill the column value when inserting data. But in the encoded value, the + // datum of this Column is missing. + // It could be a row encoded by newer schema after turning `NOT NULL` to `NULLABLE`. + // Return false to trigger schema sync when `force_decode==false`. + return false; + } + + assert(!column_info.hasNoDefaultValueFlag()); + if (!column_info.hasOriDefaultValue()) + { + // This is a Column that defined as NOT NULL with default value. In this case, tidb-server + // should fill the column value when inserting data unless the Column's default value is null, + // and the value equals to that but has no origin default. + // Reference: https://github.com/pingcap/tidb/blob/v8.5.5/pkg/table/tables/tables.go#L1463-L1489 + // Now in the encoded value, the datum of this Column is missing. It could be a row encoded by + // older schema after turning `NOT NULL` to `NULLABLE`. If the column_info has no origin default value, + // Return false to trigger schema sync when `force_decode==false`. + return false; + } + // Else the Column has a not null origin default value, the key-value should be encoded in a old schema that + // this Column is not yet added. Fallthrough to fill the column with original default value. } - // Else the row does not contain this "not null" / "no default value" column, - // it could be a row encoded by older schema. - // fallthrough to fill default value when force_decode + // Else force_decode == true, the row does not contain this "not null" / "no default value" column. + // It could be a row encoded by older schema, fallthrough to fill the column with original default value. } - // not null or has no default value, tidb will fill with specific value. + + // We consider a missing column could be safely filled with NULL or original default value. + // This could saves lots of unnecessary schema syncs for old data with a newer schema that has newly added columns. auto * raw_column = const_cast((block.getByPosition(block_column_pos)).column.get()); raw_column->insert(column_info.defaultValueToField()); return true; diff --git a/dbms/src/TiDB/Schema/TiDB.cpp b/dbms/src/TiDB/Schema/TiDB.cpp index 1054e989847..1f61e97034a 100644 --- a/dbms/src/TiDB/Schema/TiDB.cpp +++ b/dbms/src/TiDB/Schema/TiDB.cpp @@ -242,6 +242,11 @@ ColumnInfo::ColumnInfo(Poco::JSON::Object::Ptr json) } +bool ColumnInfo::hasOriDefaultValue() const +{ + return !origin_default_value.isEmpty() || !origin_default_bit_value.isEmpty(); +} + Field ColumnInfo::defaultValueToField() const { const auto & value = origin_default_value; diff --git a/dbms/src/TiDB/Schema/TiDB.h b/dbms/src/TiDB/Schema/TiDB.h index dae595255c0..86e5172c254 100644 --- a/dbms/src/TiDB/Schema/TiDB.h +++ b/dbms/src/TiDB/Schema/TiDB.h @@ -141,6 +141,7 @@ struct ColumnInfo COLUMN_FLAGS(M) #undef M + bool hasOriDefaultValue() const; DB::Field defaultValueToField() const; CodecFlag getCodecFlag() const; DB::Field getDecimalValue(const String &) const; From c28401efa798c53112b686ddf75b191cbd9d0f2d Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Tue, 27 Jan 2026 23:56:55 +0800 Subject: [PATCH 03/14] Format files Signed-off-by: JaySon-Huang --- dbms/src/TiDB/Decode/RowCodec.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/TiDB/Decode/RowCodec.cpp b/dbms/src/TiDB/Decode/RowCodec.cpp index e5ba6aa9a61..6f5305b60b2 100644 --- a/dbms/src/TiDB/Decode/RowCodec.cpp +++ b/dbms/src/TiDB/Decode/RowCodec.cpp @@ -438,7 +438,7 @@ inline bool addDefaultValueToColumnIfPossible( // Return false to trigger schema sync when `force_decode==false`. return false; } - // Else the Column has a not null origin default value, the key-value should be encoded in a old schema that + // Else the Column has a not null origin default value, the key-value should be encoded in a old schema that // this Column is not yet added. Fallthrough to fill the column with original default value. } // Else force_decode == true, the row does not contain this "not null" / "no default value" column. From 6ce06c833f0dbff0ab96db45f0695524ab1e18cc Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Wed, 28 Jan 2026 01:13:44 +0800 Subject: [PATCH 04/14] fix Signed-off-by: JaySon-Huang --- .../Storages/KVStore/tests/gtest_region_block_reader.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp b/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp index d1ab97eb1d4..d013681e541 100644 --- a/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp +++ b/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp @@ -354,9 +354,7 @@ TEST_F(RegionBlockReaderTest, MissingColumnRowV2) encodeColumns(table_info, fields, RowEncodeVersion::RowV2); auto new_table_info = getTableInfoWithMoreColumns({MutSup::extra_handle_id}, false); auto new_decoding_schema = getDecodingStorageSchemaSnapshot(new_table_info); - // `decodeAndCheckColumns` will return false because "column1" not_null=true but no_default_value=false - // FIXME: Re check the logic here - ASSERT_FALSE(decodeAndCheckColumns(new_decoding_schema, false)); + ASSERT_TRUE(decodeAndCheckColumns(new_decoding_schema, false)); } TEST_F(RegionBlockReaderTest, MissingColumnRowV1) @@ -365,9 +363,7 @@ TEST_F(RegionBlockReaderTest, MissingColumnRowV1) encodeColumns(table_info, fields, RowEncodeVersion::RowV1); auto new_table_info = getTableInfoWithMoreColumns({MutSup::extra_handle_id}, false); auto new_decoding_schema = getDecodingStorageSchemaSnapshot(new_table_info); - // `decodeAndCheckColumns` will return false because "column1" not_null=true but no_default_value=false - // FIXME: Re check the logic here - ASSERT_FALSE(decodeAndCheckColumns(new_decoding_schema, false)); + ASSERT_TRUE(decodeAndCheckColumns(new_decoding_schema, false)); } TEST_F(RegionBlockReaderTest, ExtraColumnRowV2) From 1c9a0eb7ffb46280799334cfcc827334b92e6398 Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Wed, 28 Jan 2026 01:34:05 +0800 Subject: [PATCH 05/14] Add more test assert Signed-off-by: JaySon-Huang --- .../tests/gtest_region_block_reader.cpp | 36 +++++++++++-------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp b/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp index d013681e541..01357373cd6 100644 --- a/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp +++ b/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp @@ -697,7 +697,7 @@ try { // With this table_info, c1 is filled with "0" according to ori_default TableInfo table_info( - R"({"cols":[{"id":1,"name":{"L":"c0","O":"c0"},"offset":0,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":4,"Tp":1}},{"id":2,"name":{"L":"handle","O":"handle"},"offset":1,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":515,"Flen":11,"Tp":3}},{"default":"-56083770","id":7,"name":{"L":"c1","O":"c1"},"offset":2,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":1,"Flen":20,"Tp":8}},{"id":4,"name":{"L":"c2","O":"c2"},"offset":3,"origin_default":"0.07954397","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":-1,"Flag":4097,"Flen":12,"Tp":4}},{"id":5,"name":{"L":"c5","O":"c5"},"offset":4,"origin_default":"0","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}},{"default":"247262911","id":6,"name":{"L":"c4","O":"c4"},"offset":5,"origin_default":"247262911","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}}],"id":711,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"t0","O":"t0"},"pk_is_handle":true,"state":5,"tiflash_replica":{"Available":true,"Count":1},"update_timestamp":463845180343844895})", + R"({"cols":[{"id":1,"name":{"L":"c0","O":"c0"},"offset":0,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":4,"Tp":1}},{"id":2,"name":{"L":"handle","O":"handle"},"offset":1,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":515,"Flen":11,"Tp":3}},{"default":"-56083770","id":7,"name":{"L":"c1","O":"c1"},"offset":2,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":1,"Flen":20,"Tp":8}},{"id":4,"name":{"L":"c2","O":"c2"},"offset":3,"origin_default":"0.07954397","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":-1,"Flag":4097,"Flen":12,"Tp":4}},{"id":5,"name":{"L":"c5","O":"c5"},"offset":4,"origin_default":"0","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}},{"default":"247262911","id":6,"name":{"L":"c4","O":"c4"},"offset":5,"origin_default":"247262911","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}}],"id":636,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"t0","O":"t0"},"pk_is_handle":true,"state":5,"tiflash_replica":{"Available":true,"Count":1},"update_timestamp":463845180343844895})", NullspaceID); RegionID region_id = 4; @@ -707,16 +707,16 @@ try auto region = RegionBench::makeRegionForRange(region_id, region_start_key, region_end_key); // the hex kv dump from SSTFile std::vector> kvs = { - // { - // "7480000000000002FFA95F728000000000FF0000010000000000FAF9901806DEF7FFDA", - // "50A380A08892FFF9B706762C8000040000000405060708000F0016001A00BFDC4011A00000000A0080000000000A008000003CA339" - // "1ABC85", - // }, - // { - // "7480000000000002FFA95F728000000000FF0000010000000000FAF9901806DEF7FFD8", - // "50A680A08892FFF9B706762C8000040000000405060708000F0016001A00BFDC4011A00000000A008033E04D600A008000003CA339" - // "1ABC85", - // }, + { + "7480000000000002FFA95F728000000000FF0000010000000000FAF9901806DEF7FFDA", + "50A380A08892FFF9B706762C8000040000000405060708000F0016001A00BFDC4011A00000000A0080000000000A008000003CA339" + "1ABC85", + }, + { + "7480000000000002FFA95F728000000000FF0000010000000000FAF9901806DEF7FFD8", + "50A680A08892FFF9B706762C8000040000000405060708000F0016001A00BFDC4011A00000000A008033E04D600A008000003CA339" + "1ABC85", + }, { "7480000000000002FFA95F728000000000FF0000020000000000FAF9901806DE33FFE8", "509680B08E92FFF9B706762580000300000004050608000F001600BF720CDD400000000A0080000000010A00800000393C", @@ -743,16 +743,20 @@ try auto reader = RegionBlockReader(decoding_schema); Block res_block = createBlockSortByColumnID(decoding_schema); ASSERT_TRUE(reader.read(res_block, *data_list_read, true)); - // TODO: verify the default value is filled correctly LOG_INFO( Logger::get(), "Decoded block:\n{}", DB::tests::getColumnsContent(res_block.getColumnsWithTypeAndName())); + ASSERT_EQ(res_block.getByName("c1").type->getName(), "Int64"); + // verify the default value is filled correctly + ASSERT_COLUMN_EQ( // + res_block.getByName("c1"), + createColumn({-2051270087, -2051270087, 0})); } // With this table_info, c1 does not have the "not null" flag TableInfo table_info_c1_nullable( - R"({"cols":[{"id":1,"name":{"L":"c0","O":"c0"},"offset":0,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":4,"Tp":1}},{"id":2,"name":{"L":"handle","O":"handle"},"offset":1,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":515,"Flen":11,"Tp":3}},{"id":7,"name":{"L":"c1","O":"c1"},"offset":2,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":20,"Tp":8}},{"id":4,"name":{"L":"c2","O":"c2"},"offset":3,"origin_default":"0.07954397","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":-1,"Flag":4097,"Flen":12,"Tp":4}},{"id":5,"name":{"L":"c5","O":"c5"},"offset":4,"origin_default":"0","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}},{"default":"247262911","id":6,"name":{"L":"c4","O":"c4"},"offset":5,"origin_default":"247262911","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}}],"id":681,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"t0","O":"t0"},"pk_is_handle":true,"state":5,"tiflash_replica":{"Available":true,"Count":1},"update_timestamp":463844343842340870})", + R"({"cols":[{"id":1,"name":{"L":"c0","O":"c0"},"offset":0,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":4,"Tp":1}},{"id":2,"name":{"L":"handle","O":"handle"},"offset":1,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":515,"Flen":11,"Tp":3}},{"id":7,"name":{"L":"c1","O":"c1"},"offset":2,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":20,"Tp":8}},{"id":4,"name":{"L":"c2","O":"c2"},"offset":3,"origin_default":"0.07954397","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":-1,"Flag":4097,"Flen":12,"Tp":4}},{"id":5,"name":{"L":"c5","O":"c5"},"offset":4,"origin_default":"0","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}},{"default":"247262911","id":6,"name":{"L":"c4","O":"c4"},"offset":5,"origin_default":"247262911","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}}],"id":636,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"t0","O":"t0"},"pk_is_handle":true,"state":5,"tiflash_replica":{"Available":true,"Count":1},"update_timestamp":463844343842340870})", NullspaceID); decoding_schema = getDecodingStorageSchemaSnapshot(table_info_c1_nullable); @@ -761,11 +765,15 @@ try auto reader = RegionBlockReader(decoding_schema); Block res_block = createBlockSortByColumnID(decoding_schema); ASSERT_TRUE(reader.read(res_block, *data_list_read, false)); - // TODO: verify the default value is filled correctly LOG_INFO( Logger::get(), "Decoded block:\n{}", DB::tests::getColumnsContent(res_block.getColumnsWithTypeAndName())); + ASSERT_EQ(res_block.getByName("c1").type->getName(), "Nullable(Int64)"); + // verify the default value is filled with NULL correctly at the last row + ASSERT_COLUMN_EQ( // + res_block.getByName("c1"), + createNullableColumn({-2051270087, -2051270087, 0}, {0, 0, 1})); } } CATCH From d7c8130eae903578748e8ce6fa689a885c08d78d Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Wed, 28 Jan 2026 12:20:30 +0800 Subject: [PATCH 06/14] Clearly comment Signed-off-by: JaySon-Huang --- dbms/src/TiDB/Decode/RowCodec.cpp | 71 +++++++++++++------------------ dbms/src/TiDB/Schema/TiDB.cpp | 14 +++++- dbms/src/TiDB/Schema/TiDB.h | 2 +- 3 files changed, 44 insertions(+), 43 deletions(-) diff --git a/dbms/src/TiDB/Decode/RowCodec.cpp b/dbms/src/TiDB/Decode/RowCodec.cpp index 6f5305b60b2..7291d68d07e 100644 --- a/dbms/src/TiDB/Decode/RowCodec.cpp +++ b/dbms/src/TiDB/Decode/RowCodec.cpp @@ -388,8 +388,20 @@ bool appendRowToBlock( } } -// When the `column_info` is missing in the encoded row, we try to add default value to the `block` at `block_column_pos`. -// Return true if we could add default value to the column. Otherwise false and the caller should trigger schema sync. +// When a column is missing in the encoded row, decide whether we can append a value +// (NULL or origin default) to `block` or we must trigger schema sync. +// +// Background +// - TiDB encode semantics, see [tables.CanSkip](https://github.com/pingcap/tidb/blob/v8.5.5/pkg/table/tables/tables.go#L1463-L1489) +// - PK handle columns may be omitted from the value part and decoded from the key. +// - A NULL column may be omitted only when BOTH DefaultValue and OriginDefaultValue are empty. +// - For NOT NULL columns, TiDB must encode the value in the row unless the column did not +// exist in the writer schema (old data); in that case OriginDefaultValue is expected. +// +// Policy here: +// - If the omission is clearly valid under the current schema, we fill a value and return true. +// - Otherwise return false (force_decode == false) to let the caller sync schema and retry. +// - If force_decode == true, we fall back to best-effort filling. inline bool addDefaultValueToColumnIfPossible( const ColumnInfo & column_info, Block & block, @@ -397,57 +409,34 @@ inline bool addDefaultValueToColumnIfPossible( bool ignore_pk_if_absent, bool force_decode) { + // 1) Primary-key columns can be decoded from the key for pk_is_handle / common-handle tables. + // Skip value-part filling here if allowed. if (column_info.hasPriKeyFlag()) { - // For clustered index or pk_is_handle, if the pk column does not exists, it can still be decoded from the key. - // just skip this column. if (ignore_pk_if_absent) return true; - assert(!ignore_pk_if_absent); + // For non-clustered tables, a missing PK column implies schema mismatch. if (!force_decode) return false; - // Else non-clustered index, and not pk_is_handle, it could be a row encoded by older schema, - // we need to fill the column which has primary key flag with default value. - // fallthrough to fill default value when `force_decode==true` + // fallthrough for best-effort fill when force_decode == true } - if (column_info.hasNotNullFlag()) + // 2) NOT NULL columns: + // - If the column has NO DEFAULT, TiDB should always encode a value. Missing datum implies mismatch. + // - If the column has NO origin default, missing datum may come from a newer schema where the column + // became NULLABLE and was skipped; require schema sync. + // - If origin default exists, missing datum can be from older rows before the column was added; safe to fill. + if (!force_decode && column_info.hasNotNullFlag()) { - if (!force_decode) - { - if (column_info.hasNoDefaultValueFlag()) - { - // This is a Column that defined as NOT NULL but no default value. In this case, user - // should fill the column value when inserting data. But in the encoded value, the - // datum of this Column is missing. - // It could be a row encoded by newer schema after turning `NOT NULL` to `NULLABLE`. - // Return false to trigger schema sync when `force_decode==false`. - return false; - } - - assert(!column_info.hasNoDefaultValueFlag()); - if (!column_info.hasOriDefaultValue()) - { - // This is a Column that defined as NOT NULL with default value. In this case, tidb-server - // should fill the column value when inserting data unless the Column's default value is null, - // and the value equals to that but has no origin default. - // Reference: https://github.com/pingcap/tidb/blob/v8.5.5/pkg/table/tables/tables.go#L1463-L1489 - // Now in the encoded value, the datum of this Column is missing. It could be a row encoded by - // older schema after turning `NOT NULL` to `NULLABLE`. If the column_info has no origin default value, - // Return false to trigger schema sync when `force_decode==false`. - return false; - } - // Else the Column has a not null origin default value, the key-value should be encoded in a old schema that - // this Column is not yet added. Fallthrough to fill the column with original default value. - } - // Else force_decode == true, the row does not contain this "not null" / "no default value" column. - // It could be a row encoded by older schema, fallthrough to fill the column with original default value. + if (column_info.hasNoDefaultValueFlag() || !column_info.hasOriginDefaultValue()) + return false; } - // We consider a missing column could be safely filled with NULL or original default value. - // This could saves lots of unnecessary schema syncs for old data with a newer schema that has newly added columns. - auto * raw_column = const_cast((block.getByPosition(block_column_pos)).column.get()); + // 3) Fill using origin default or NULL. + // Note: defaultValueToField() uses origin_default_value/origin_default_bit_value, + // and falls back to NULL (nullable) or GenDefaultField (NOT NULL) when they are empty. + auto * raw_column = const_cast(block.getByPosition(block_column_pos).column.get()); raw_column->insert(column_info.defaultValueToField()); return true; } diff --git a/dbms/src/TiDB/Schema/TiDB.cpp b/dbms/src/TiDB/Schema/TiDB.cpp index 1f61e97034a..2afbf157c29 100644 --- a/dbms/src/TiDB/Schema/TiDB.cpp +++ b/dbms/src/TiDB/Schema/TiDB.cpp @@ -242,11 +242,23 @@ ColumnInfo::ColumnInfo(Poco::JSON::Object::Ptr json) } -bool ColumnInfo::hasOriDefaultValue() const +bool ColumnInfo::hasOriginDefaultValue() const { + // Whether `origin_default_value` or `origin_default_bit_value` is set. return !origin_default_value.isEmpty() || !origin_default_bit_value.isEmpty(); } +// Convert the column's *origin default* into a Field for storage decoding/backfill. +// +// Note: TiFlash intentionally uses origin_default_value/origin_default_bit_value +// instead of ColumnInfo::default_value, because TiDB’s encoding/CanSkip logic is +// based on origin defaults. +// +// Semantics: +// - If both origin defaults are empty and the column is nullable -> return NULL Field. +// - If both are empty and the column is NOT NULL -> return GenDefaultField (type-specific zero). +// - Otherwise parse/convert the origin default according to column type +// (time/decimal/enum/set/bit/json/etc.). Field ColumnInfo::defaultValueToField() const { const auto & value = origin_default_value; diff --git a/dbms/src/TiDB/Schema/TiDB.h b/dbms/src/TiDB/Schema/TiDB.h index 86e5172c254..6dc58376b8e 100644 --- a/dbms/src/TiDB/Schema/TiDB.h +++ b/dbms/src/TiDB/Schema/TiDB.h @@ -141,7 +141,7 @@ struct ColumnInfo COLUMN_FLAGS(M) #undef M - bool hasOriDefaultValue() const; + bool hasOriginDefaultValue() const; DB::Field defaultValueToField() const; CodecFlag getCodecFlag() const; DB::Field getDecimalValue(const String &) const; From c8c38759779691f92b6f574e4a634e20268747ee Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Wed, 28 Jan 2026 12:57:27 +0800 Subject: [PATCH 07/14] Add table info parsing ut Signed-off-by: JaySon-Huang --- dbms/src/TiDB/Schema/tests/gtest_table_info.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/dbms/src/TiDB/Schema/tests/gtest_table_info.cpp b/dbms/src/TiDB/Schema/tests/gtest_table_info.cpp index cac0bad7608..f14c9c88cbc 100644 --- a/dbms/src/TiDB/Schema/tests/gtest_table_info.cpp +++ b/dbms/src/TiDB/Schema/tests/gtest_table_info.cpp @@ -67,6 +67,8 @@ try [](const TableInfo & table_info) { ASSERT_EQ(table_info.name, "t"); ASSERT_EQ(table_info.id, 45L); + ASSERT_EQ(table_info.columns.size(), 1U); + ASSERT_FALSE(table_info.columns[0].hasOriginDefaultValue()); }}, // Test with tiflash_replica information ParseCase{ @@ -79,6 +81,8 @@ try ParseCase{ R"json({"id":45,"name":{"O":"t","L":"t"},"charset":"utf8mb4","collate":"utf8mb4_bin","cols":[{"id":1,"name":{"O":"t","L":"t"},"offset":0,"origin_default":"\u0000\u00124","origin_default_bit":null,"default":null,"default_bit":null,"default_is_expr":false,"generated_expr_string":"","generated_stored":false,"dependences":null,"type":{"Tp":254,"Flag":129,"Flen":4,"Decimal":0,"Charset":"binary","Collate":"binary","Elems":null},"state":5,"comment":"","hidden":false,"change_state_info":null,"version":2}],"index_info":null,"constraint_info":null,"fk_info":null,"state":5,"pk_is_handle":false,"is_common_handle":false,"comment":"","auto_inc_id":0,"auto_id_cache":0,"auto_rand_id":0,"max_col_id":1,"max_idx_id":0,"max_cst_id":0,"update_timestamp":418683341902184450,"ShardRowIDBits":0,"max_shard_row_id_bits":0,"auto_random_bits":0,"pre_split_regions":0,"partition":null,"compression":"","view":null,"sequence":null,"Lock":null,"version":3,"tiflash_replica":{"Count":1,"LocationLabels":[],"Available":false,"AvailablePartitionIDs":null}})json", [](const TableInfo & table_info) { + ASSERT_EQ(table_info.columns.size(), 1U); + ASSERT_EQ(table_info.columns[0].hasOriginDefaultValue(), true); ASSERT_EQ( table_info.columns[0].defaultValueToField().get(), Field(String( @@ -91,6 +95,8 @@ try ParseCase{ R"json({"id":45,"name":{"O":"t","L":"t"},"charset":"utf8mb4","collate":"utf8mb4_bin","cols":[{"id":1,"name":{"O":"t","L":"t"},"offset":0,"origin_default":"\u0000\u00124","origin_default_bit":null,"default":null,"default_bit":null,"default_is_expr":false,"generated_expr_string":"","generated_stored":false,"dependences":null,"type":{"Tp":254,"Flag":129,"Flen":3,"Decimal":0,"Charset":"binary","Collate":"binary","Elems":null},"state":5,"comment":"","hidden":false,"change_state_info":null,"version":2}],"index_info":null,"constraint_info":null,"fk_info":null,"state":5,"pk_is_handle":false,"is_common_handle":false,"comment":"","auto_inc_id":0,"auto_id_cache":0,"auto_rand_id":0,"max_col_id":1,"max_idx_id":0,"max_cst_id":0,"update_timestamp":418683341902184450,"ShardRowIDBits":0,"max_shard_row_id_bits":0,"auto_random_bits":0,"pre_split_regions":0,"partition":null,"compression":"","view":null,"sequence":null,"Lock":null,"version":3,"tiflash_replica":{"Count":1,"LocationLabels":[],"Available":false,"AvailablePartitionIDs":null}})json", [](const TableInfo & table_info) { + ASSERT_EQ(table_info.columns.size(), 1U); + ASSERT_EQ(table_info.columns[0].hasOriginDefaultValue(), true); ASSERT_EQ( table_info.columns[0].defaultValueToField().get(), Field(String( @@ -148,6 +154,7 @@ try auto col_3 = table_info.getColumnInfo(3); ASSERT_EQ(col_3.tp, TiDB::TP::TypeVarchar); ASSERT_TRUE(col_3.hasNotNullFlag()); + ASSERT_TRUE(col_3.hasOriginDefaultValue()); // The "origin_default" is "100", which is used for filling default value for old rows that is inserted before this column is added. ASSERT_EQ(col_3.defaultValueToField().get(), "100"); }}, From 817ce951f8577380bc98d55a708c2c0cbfb84e22 Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Wed, 28 Jan 2026 13:21:16 +0800 Subject: [PATCH 08/14] Add more case in ReadFromRegionDefaultValue Signed-off-by: JaySon-Huang --- .../tests/gtest_region_block_reader.cpp | 72 ++++++++++++++++--- dbms/src/TiDB/Decode/RowCodec.cpp | 2 + 2 files changed, 66 insertions(+), 8 deletions(-) diff --git a/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp b/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp index 01357373cd6..42c299c83ff 100644 --- a/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp +++ b/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp @@ -695,17 +695,32 @@ CATCH TEST_F(RegionBlockReaderTest, ReadFromRegionDefaultValue) try { - // With this table_info, c1 is filled with "0" according to ori_default - TableInfo table_info( + // With this table_info, column "c1" is "NOT NULL" and has no origin default + TableInfo table_info_c1_not_null_no_origin_default( R"({"cols":[{"id":1,"name":{"L":"c0","O":"c0"},"offset":0,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":4,"Tp":1}},{"id":2,"name":{"L":"handle","O":"handle"},"offset":1,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":515,"Flen":11,"Tp":3}},{"default":"-56083770","id":7,"name":{"L":"c1","O":"c1"},"offset":2,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":1,"Flen":20,"Tp":8}},{"id":4,"name":{"L":"c2","O":"c2"},"offset":3,"origin_default":"0.07954397","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":-1,"Flag":4097,"Flen":12,"Tp":4}},{"id":5,"name":{"L":"c5","O":"c5"},"offset":4,"origin_default":"0","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}},{"default":"247262911","id":6,"name":{"L":"c4","O":"c4"},"offset":5,"origin_default":"247262911","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}}],"id":636,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"t0","O":"t0"},"pk_is_handle":true,"state":5,"tiflash_replica":{"Available":true,"Count":1},"update_timestamp":463845180343844895})", NullspaceID); + // With this table_info, column "c1" is "NOT NULL" and has no default value + TableInfo table_info_c1_not_null_no_default_value( + R"({"cols":[{"id":1,"name":{"L":"c0","O":"c0"},"offset":0,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":4,"Tp":1}},{"id":2,"name":{"L":"handle","O":"handle"},"offset":1,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":515,"Flen":11,"Tp":3}},{"default":"-56083770","id":7,"name":{"L":"c1","O":"c1"},"offset":2,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":4097,"Flen":20,"Tp":8}},{"id":4,"name":{"L":"c2","O":"c2"},"offset":3,"origin_default":"0.07954397","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":-1,"Flag":4097,"Flen":12,"Tp":4}},{"id":5,"name":{"L":"c5","O":"c5"},"offset":4,"origin_default":"0","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}},{"default":"247262911","id":6,"name":{"L":"c4","O":"c4"},"offset":5,"origin_default":"247262911","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}}],"id":636,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"t0","O":"t0"},"pk_is_handle":true,"state":5,"tiflash_replica":{"Available":true,"Count":1},"update_timestamp":463845180343844895})", + NullspaceID); + + // With this table_info, column "c1" has the "NOT NULL" flag and has origin default "-56083770" + TableInfo table_info_c1_not_null_with_origin_default( + R"({"cols":[{"id":1,"name":{"L":"c0","O":"c0"},"offset":0,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":4,"Tp":1}},{"id":2,"name":{"L":"handle","O":"handle"},"offset":1,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":515,"Flen":11,"Tp":3}},{"origin_default":"-56083770","id":7,"name":{"L":"c1","O":"c1"},"offset":2,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":1,"Flen":20,"Tp":8}},{"id":4,"name":{"L":"c2","O":"c2"},"offset":3,"origin_default":"0.07954397","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":-1,"Flag":4097,"Flen":12,"Tp":4}},{"id":5,"name":{"L":"c5","O":"c5"},"offset":4,"origin_default":"0","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}},{"default":"247262911","id":6,"name":{"L":"c4","O":"c4"},"offset":5,"origin_default":"247262911","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}}],"id":636,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"t0","O":"t0"},"pk_is_handle":true,"state":5,"tiflash_replica":{"Available":true,"Count":1},"update_timestamp":463845180343844895})", + NullspaceID); + + // With this table_info, column "c1" does not have the "NOT NULL" flag + TableInfo table_info_c1_nullable( + R"({"cols":[{"id":1,"name":{"L":"c0","O":"c0"},"offset":0,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":4,"Tp":1}},{"id":2,"name":{"L":"handle","O":"handle"},"offset":1,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":515,"Flen":11,"Tp":3}},{"id":7,"name":{"L":"c1","O":"c1"},"offset":2,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":20,"Tp":8}},{"id":4,"name":{"L":"c2","O":"c2"},"offset":3,"origin_default":"0.07954397","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":-1,"Flag":4097,"Flen":12,"Tp":4}},{"id":5,"name":{"L":"c5","O":"c5"},"offset":4,"origin_default":"0","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}},{"default":"247262911","id":6,"name":{"L":"c4","O":"c4"},"offset":5,"origin_default":"247262911","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}}],"id":636,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"t0","O":"t0"},"pk_is_handle":true,"state":5,"tiflash_replica":{"Available":true,"Count":1},"update_timestamp":463844343842340870})", + NullspaceID); + RegionID region_id = 4; // the start_key and end_key for table_id = 68 String region_start_key(bytesFromHexString("7480000000000002FF7C5F720000000000FA")); String region_end_key(bytesFromHexString("7480000000000002FF7D00000000000000F8")); auto region = RegionBench::makeRegionForRange(region_id, region_start_key, region_end_key); - // the hex kv dump from SSTFile + // the hex kv dump from RaftLog std::vector> kvs = { { "7480000000000002FFA95F728000000000FF0000010000000000FAF9901806DEF7FFDA", @@ -730,7 +745,8 @@ try auto data_list_read = ReadRegionCommitCache(region, true); ASSERT_TRUE(data_list_read.has_value()); - auto decoding_schema = getDecodingStorageSchemaSnapshot(table_info); + // Test with `table_info_c1_not_null_no_origin_default` + auto decoding_schema = getDecodingStorageSchemaSnapshot(table_info_c1_not_null_no_origin_default); { // force_decode=false can not decode because there are // missing value for column with not null flag. @@ -754,11 +770,51 @@ try createColumn({-2051270087, -2051270087, 0})); } - // With this table_info, c1 does not have the "not null" flag - TableInfo table_info_c1_nullable( - R"({"cols":[{"id":1,"name":{"L":"c0","O":"c0"},"offset":0,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":4,"Tp":1}},{"id":2,"name":{"L":"handle","O":"handle"},"offset":1,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":515,"Flen":11,"Tp":3}},{"id":7,"name":{"L":"c1","O":"c1"},"offset":2,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":20,"Tp":8}},{"id":4,"name":{"L":"c2","O":"c2"},"offset":3,"origin_default":"0.07954397","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":-1,"Flag":4097,"Flen":12,"Tp":4}},{"id":5,"name":{"L":"c5","O":"c5"},"offset":4,"origin_default":"0","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}},{"default":"247262911","id":6,"name":{"L":"c4","O":"c4"},"offset":5,"origin_default":"247262911","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}}],"id":636,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"t0","O":"t0"},"pk_is_handle":true,"state":5,"tiflash_replica":{"Available":true,"Count":1},"update_timestamp":463844343842340870})", - NullspaceID); + // Test with `table_info_c1_not_null_no_default_value` + decoding_schema = getDecodingStorageSchemaSnapshot(table_info_c1_not_null_no_default_value); + { + // force_decode=false can not decode because there are + // missing value for column with not null flag. + auto reader = RegionBlockReader(decoding_schema); + Block res_block = createBlockSortByColumnID(decoding_schema); + ASSERT_FALSE(reader.read(res_block, *data_list_read, false)); + } + { + // force_decode=true can decode the block, and filling the default value for c1 + auto reader = RegionBlockReader(decoding_schema); + Block res_block = createBlockSortByColumnID(decoding_schema); + ASSERT_TRUE(reader.read(res_block, *data_list_read, true)); + LOG_INFO( + Logger::get(), + "Decoded block:\n{}", + DB::tests::getColumnsContent(res_block.getColumnsWithTypeAndName())); + ASSERT_EQ(res_block.getByName("c1").type->getName(), "Int64"); + // verify the default value is filled correctly + ASSERT_COLUMN_EQ( // + res_block.getByName("c1"), + createColumn({-2051270087, -2051270087, 0})); + } + + // Test with `table_info_c1_not_null_with_origin_default` + decoding_schema = getDecodingStorageSchemaSnapshot(table_info_c1_not_null_with_origin_default); + { + // force_decode=false can decode because origin_default exists and NoDefaultValue flag is not set + // so RegionBlockReader can use origin_default to fill the missing value` + auto reader = RegionBlockReader(decoding_schema); + Block res_block = createBlockSortByColumnID(decoding_schema); + ASSERT_TRUE(reader.read(res_block, *data_list_read, false)); + LOG_INFO( + Logger::get(), + "Decoded block:\n{}", + DB::tests::getColumnsContent(res_block.getColumnsWithTypeAndName())); + ASSERT_EQ(res_block.getByName("c1").type->getName(), "Int64"); + // verify the default value is filled correctly + ASSERT_COLUMN_EQ( // + res_block.getByName("c1"), + createColumn({-2051270087, -2051270087, -56083770})); + } + // Test with `table_info_c1_nullable` decoding_schema = getDecodingStorageSchemaSnapshot(table_info_c1_nullable); { // force_decode=false should be able to decode because c1 is nullable diff --git a/dbms/src/TiDB/Decode/RowCodec.cpp b/dbms/src/TiDB/Decode/RowCodec.cpp index 7291d68d07e..91a00951424 100644 --- a/dbms/src/TiDB/Decode/RowCodec.cpp +++ b/dbms/src/TiDB/Decode/RowCodec.cpp @@ -524,6 +524,7 @@ bool appendRowV2ToBlockImpl( ignore_pk_if_absent, force_decode)) { + // If failed to fill default value, return false to let upper layer trigger schema sync. return false; } column_ids_iter++; @@ -606,6 +607,7 @@ bool appendRowV2ToBlockImpl( ignore_pk_if_absent, force_decode)) { + // If failed to fill default value, return false to let upper layer trigger schema sync. return false; } } From bc5ae8792d496d02166485300869056fcbee66a1 Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Wed, 28 Jan 2026 15:37:38 +0800 Subject: [PATCH 09/14] Add fullstack test Signed-off-by: JaySon-Huang --- .../ddl/alter_default_value.test | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/tests/fullstack-test2/ddl/alter_default_value.test b/tests/fullstack-test2/ddl/alter_default_value.test index a0385362f2b..041bc043ab0 100644 --- a/tests/fullstack-test2/ddl/alter_default_value.test +++ b/tests/fullstack-test2/ddl/alter_default_value.test @@ -130,3 +130,64 @@ mysql> set session tidb_isolation_read_engines='tiflash'; select col1, col2, col | 30 | 0000 | 0.00 | 1.23 | 123 | sss | 2017-02-11 00:00:00 | 2018-11-04 23:12:03 | 2012 | value3 | value1 | 2018-11-04 23:12:03 | 12:18:30 | NULL | 10203 | +------+------+------+------+------+------+---------------------+---------------------+------+--------+--------+---------------------+----------+-------+------------+ mysql> drop table if exists test.t + +############# +# issue 10680, TiFlash data inconsistent with TiKV after modifying a column from NOT NULL to NULL +mysql> DROP DATABASE IF EXISTS db_1483421321; +mysql> CREATE DATABASE db_1483421321; +mysql> CREATE TABLE db_1483421321.t0(c0 TINYINT, handle INT NOT NULL AUTO_INCREMENT, PRIMARY KEY(handle)); +mysql> ALTER TABLE db_1483421321.t0 SET TIFLASH REPLICA 1; +func> wait_table db_1483421321 t0 +# empty table +mysql> set session tidb_isolation_read_engines='tiflash'; SELECT * FROM db_1483421321.t0 order by handle; +mysql> set session tidb_isolation_read_engines='tikv'; SELECT * FROM db_1483421321.t0 order by handle; + +mysql> ALTER TABLE db_1483421321.t0 ADD COLUMN c1 DECIMAL NULL; +mysql> ALTER TABLE db_1483421321.t0 ADD COLUMN c2 FLOAT NULL DEFAULT 0.0795439693286002; +mysql> ALTER TABLE db_1483421321.t0 ADD COLUMN c5 DECIMAL NOT NULL; +mysql> ALTER TABLE db_1483421321.t0 ADD COLUMN c4 DECIMAL DEFAULT 247262911; +mysql> ALTER TABLE db_1483421321.t0 MODIFY COLUMN c2 FLOAT NOT NULL; +mysql> ALTER TABLE db_1483421321.t0 MODIFY COLUMN c5 DECIMAL NULL; +mysql> ALTER TABLE db_1483421321.t0 MODIFY COLUMN c1 BIGINT DEFAULT -56083770 NOT NULL; + +# first insert +mysql> INSERT IGNORE INTO db_1483421321.t0 (c1, c2, c5, c4) VALUES (-2051270087, 0.44141045099028775, 0.0, 15523); +# update and generate a new raftlog +mysql> UPDATE db_1483421321.t0 SET c5 = 870337888; +mysql> ALTER TABLE db_1483421321.t0 MODIFY COLUMN c1 BIGINT NULL; + +# insert and generate the third raftlog +mysql> INSERT INTO db_1483421321.t0 (c2, c5, c4) VALUES (0.004406799693866592, 0.8752311290235516, 14652); +mysql> set session tidb_isolation_read_engines='tiflash'; SELECT * FROM db_1483421321.t0 order by handle; ++------+--------+-------------+--------------+-----------+-------+ +| c0 | handle | c1 | c2 | c5 | c4 | ++------+--------+-------------+--------------+-----------+-------+ +| NULL | 1 | -2051270087 | 0.44141045 | 870337888 | 15523 | +| NULL | 2 | NULL | 0.0044067996 | 1 | 14652 | ++------+--------+-------------+--------------+-----------+-------+ +mysql> set session tidb_isolation_read_engines='tikv'; SELECT * FROM db_1483421321.t0 order by handle; ++------+--------+-------------+--------------+-----------+-------+ +| c0 | handle | c1 | c2 | c5 | c4 | ++------+--------+-------------+--------------+-----------+-------+ +| NULL | 1 | -2051270087 | 0.44141045 | 870337888 | 15523 | +| NULL | 2 | NULL | 0.0044067996 | 1 | 14652 | ++------+--------+-------------+--------------+-----------+-------+ + +# more insert +mysql> INSERT INTO db_1483421321.t0 (c2, c5, c4) VALUES (0.004406799693866592, 0.8752311290235516, 14652); +mysql> set session tidb_isolation_read_engines='tiflash'; SELECT * FROM db_1483421321.t0 order by handle; ++------+--------+-------------+--------------+-----------+-------+ +| c0 | handle | c1 | c2 | c5 | c4 | ++------+--------+-------------+--------------+-----------+-------+ +| NULL | 1 | -2051270087 | 0.44141045 | 870337888 | 15523 | +| NULL | 2 | NULL | 0.0044067996 | 1 | 14652 | +| NULL | 3 | NULL | 0.0044067996 | 1 | 14652 | ++------+--------+-------------+--------------+-----------+-------+ +mysql> set session tidb_isolation_read_engines='tikv'; SELECT * FROM db_1483421321.t0 order by handle; ++------+--------+-------------+--------------+-----------+-------+ +| c0 | handle | c1 | c2 | c5 | c4 | ++------+--------+-------------+--------------+-----------+-------+ +| NULL | 1 | -2051270087 | 0.44141045 | 870337888 | 15523 | +| NULL | 2 | NULL | 0.0044067996 | 1 | 14652 | +| NULL | 3 | NULL | 0.0044067996 | 1 | 14652 | ++------+--------+-------------+--------------+-----------+-------+ From d8448b5cf013e3c43d0561ddc45278c2edf049d6 Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Mon, 26 Jan 2026 19:44:18 +0800 Subject: [PATCH 10/14] Add debug log and test case Signed-off-by: JaySon-Huang --- .../KVStore/Decode/PartitionStreams.cpp | 12 ++++++ .../KVStore/Decode/RegionBlockReader.cpp | 10 +++++ .../KVStore/MultiRaft/RaftCommands.cpp | 1 + dbms/src/Storages/StorageDeltaMerge.cpp | 8 ++++ dbms/src/TiDB/Decode/RowCodec.cpp | 37 +++++++++++++++++++ 5 files changed, 68 insertions(+) diff --git a/dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp b/dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp index 4e47246b305..31f445fbe1f 100644 --- a/dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp +++ b/dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp @@ -159,6 +159,18 @@ static inline bool atomicReadWrite( std::tie(decoding_schema_snapshot, block_ptr) = storage->getSchemaSnapshotAndBlockForDecoding(lock, true, should_handle_version_col); block_decoding_schema_epoch = decoding_schema_snapshot->decoding_schema_epoch; + const auto & table_info = storage->getTableInfo(); + LOG_INFO( + Logger::get("dddddddddd"), + "decode with schema snapshot, keyspace={} table_id={} region_id={} epoch={} update_ts={} " + "with_version_column={} columns={}", + rw_ctx.keyspace_id, + rw_ctx.table_id, + region->id(), + block_decoding_schema_epoch, + table_info.update_timestamp, + should_handle_version_col, + table_info.columns.size()); auto reader = RegionBlockReader(decoding_schema_snapshot); if (!reader.read(*block_ptr, data_list_read, force_decode)) diff --git a/dbms/src/Storages/KVStore/Decode/RegionBlockReader.cpp b/dbms/src/Storages/KVStore/Decode/RegionBlockReader.cpp index 02706d6ac17..9a1696abaa0 100644 --- a/dbms/src/Storages/KVStore/Decode/RegionBlockReader.cpp +++ b/dbms/src/Storages/KVStore/Decode/RegionBlockReader.cpp @@ -44,6 +44,11 @@ bool RegionBlockReader::read(Block & block, const ReadList & data_list, bool for { try { + for (const auto & data : data_list) + { + LOG_INFO(Logger::get("dddddddddd"), "RegionBlockReader::read data.value={}", data.value->toDebugString()); + } + switch (schema_snapshot->pk_type) { case TMTPKType::INT64: @@ -177,6 +182,11 @@ struct VersionColResolver template bool RegionBlockReader::readImpl(Block & block, const ReadList & data_list, bool force_decode) { + LOG_INFO( + Logger::get("dddddddddd"), + "RegionBlockReader::readImpl start, schema_snapshot.column_defines={}", + *schema_snapshot->column_defines); + VersionColResolver version_col_resolver; version_col_resolver.check(block, schema_snapshot->column_defines->size()); // The column_ids to read according to schema_snapshot, each elem is (column_id, block_pos) diff --git a/dbms/src/Storages/KVStore/MultiRaft/RaftCommands.cpp b/dbms/src/Storages/KVStore/MultiRaft/RaftCommands.cpp index bfb9d2b06e7..e2e05ce25f5 100644 --- a/dbms/src/Storages/KVStore/MultiRaft/RaftCommands.cpp +++ b/dbms/src/Storages/KVStore/MultiRaft/RaftCommands.cpp @@ -368,6 +368,7 @@ std::pair Region::handleWriteRaftCmd( { auto tikv_key = TiKVKey(cmds.keys[i].data, cmds.keys[i].len); auto tikv_value = TiKVValue(cmds.vals[i].data, cmds.vals[i].len); + LOG_INFO(Logger::get("dddddddddd"), "Region::handleWriteRaftCmd Put key={}, value={}", tikv_key.toDebugString(), tikv_value.toDebugString()); if (cf == ColumnFamilyType::Write) { write_put_key_count++; diff --git a/dbms/src/Storages/StorageDeltaMerge.cpp b/dbms/src/Storages/StorageDeltaMerge.cpp index 14926b90e09..7ac3f543642 100644 --- a/dbms/src/Storages/StorageDeltaMerge.cpp +++ b/dbms/src/Storages/StorageDeltaMerge.cpp @@ -1254,6 +1254,14 @@ std::pair StorageDeltaMerg store->getHandle(), decoding_schema_epoch++, with_version_column); + LOG_INFO( + Logger::get("dddddddddd"), + "Refresh decoding schema snapshot, table_id={} epoch={} update_ts={} with_version_column={} table_info={}", + tidb_table_info.id, + decoding_schema_snapshot->decoding_schema_epoch, + tidb_table_info.update_timestamp, + with_version_column, + tidb_table_info.serialize()); cache_blocks.clear(); decoding_schema_changed = false; } diff --git a/dbms/src/TiDB/Decode/RowCodec.cpp b/dbms/src/TiDB/Decode/RowCodec.cpp index 91a00951424..180698135d7 100644 --- a/dbms/src/TiDB/Decode/RowCodec.cpp +++ b/dbms/src/TiDB/Decode/RowCodec.cpp @@ -13,6 +13,7 @@ // limitations under the License. #include +#include #include #include #include @@ -474,6 +475,12 @@ bool appendRowV2ToBlockImpl( raw_value, num_not_null_columns, value_offsets); + LOG_INFO( + Logger::get("dddddddddd"), + "not_null_column_ids={} null_column_ids={}", + not_null_column_ids, + null_column_ids); + size_t values_start_pos = cursor; // how many not null columns have been processed size_t idx_not_null = 0; @@ -496,6 +503,12 @@ bool appendRowV2ToBlockImpl( auto next_datum_column_id = is_null ? null_column_ids[idx_null] : not_null_column_ids[idx_not_null]; const auto next_column_id = column_ids_iter->first; + LOG_INFO( + Logger::get("dddddddddd"), + "next_column_id={} next_datum_column_id={} force_decode={}", + next_column_id, + next_datum_column_id, + force_decode); if (next_column_id > next_datum_column_id) { // The next_column_id to read is bigger than the next_datum_column_id in encoded row. @@ -517,6 +530,19 @@ bool appendRowV2ToBlockImpl( // a column. // Fill with default value and continue to read data for next column id. const auto & column_info = column_infos[column_ids_iter->second]; + LOG_INFO( + Logger::get("dddddddddd"), + "appendRowV2ToBlockImpl: fill default value for missing column," + " next_column_id={} next_datum_column_id={} block_column_pos={}" + " column_info={{name={} id={} not_null={} no_default_val={} default_value={}}}", + next_column_id, + next_datum_column_id, + block_column_pos, + column_info.name, + column_info.id, + column_info.hasNotNullFlag(), + column_info.hasNoDefaultValueFlag(), + applyVisitor(FieldVisitorToString(), column_info.defaultValueToField())); if (!addDefaultValueToColumnIfPossible( column_info, block, @@ -600,6 +626,17 @@ bool appendRowV2ToBlockImpl( if (column_ids_iter->first != pk_handle_id) { const auto & column_info = column_infos[column_ids_iter->second]; + LOG_INFO( + Logger::get("dddddddddd"), + "appendRowV2ToBlockImpl: fill default value for missing column," + " block_column_pos={}" + " column_info={{name={} id={} not_null={} no_default_val={} default_value={}}}", + block_column_pos, + column_info.name, + column_info.id, + column_info.hasNotNullFlag(), + column_info.hasNoDefaultValueFlag(), + applyVisitor(FieldVisitorToString(), column_info.defaultValueToField())); if (!addDefaultValueToColumnIfPossible( column_info, block, From 02761a85901076f8aafd8e4c8cfa719881ce48ee Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Wed, 28 Jan 2026 20:43:48 +0800 Subject: [PATCH 11/14] Add the case to handle column under non-public state with origin_default Signed-off-by: JaySon-Huang --- dbms/src/TiDB/Decode/RowCodec.cpp | 13 +- .../ddl/alter_column_nullable.test | 121 ++++++++++++++++++ .../ddl/alter_default_value.test | 60 --------- 3 files changed, 133 insertions(+), 61 deletions(-) diff --git a/dbms/src/TiDB/Decode/RowCodec.cpp b/dbms/src/TiDB/Decode/RowCodec.cpp index 180698135d7..6ce5b376c68 100644 --- a/dbms/src/TiDB/Decode/RowCodec.cpp +++ b/dbms/src/TiDB/Decode/RowCodec.cpp @@ -427,10 +427,21 @@ inline bool addDefaultValueToColumnIfPossible( // - If the column has NO DEFAULT, TiDB should always encode a value. Missing datum implies mismatch. // - If the column has NO origin default, missing datum may come from a newer schema where the column // became NULLABLE and was skipped; require schema sync. + // - If the column state is NOT public, missing datum may come from a newer schema where the column + // became PUBLIC and then became NULLABLE, the datum was skipped; require schema sync. // - If origin default exists, missing datum can be from older rows before the column was added; safe to fill. + // + // Note that for a non-public column, TiDB could use the `origin_default_value` to fill missing datum for backfilling + // during schema change. And before the column state becomes public, TiDB will reset the `origin_default_value` to null. + // So when all following conditions are met, it implies schema mismatch and TiFlash should require schema sync: + // - the column datum is missing + // - the column has not null flag + // - the column state is not public if (!force_decode && column_info.hasNotNullFlag()) { - if (column_info.hasNoDefaultValueFlag() || !column_info.hasOriginDefaultValue()) + if (column_info.hasNoDefaultValueFlag() // + || column_info.state != TiDB::SchemaState::StatePublic // + || !column_info.hasOriginDefaultValue()) return false; } diff --git a/tests/fullstack-test2/ddl/alter_column_nullable.test b/tests/fullstack-test2/ddl/alter_column_nullable.test index 3afd95cd2bd..ca7445bf5f8 100644 --- a/tests/fullstack-test2/ddl/alter_column_nullable.test +++ b/tests/fullstack-test2/ddl/alter_column_nullable.test @@ -144,3 +144,124 @@ mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.alt +------+---+----+ mysql> drop table if exists test.alt2 + +############# +# issue 10680, TiFlash data inconsistent with TiKV after modifying a column from NOT NULL to NULL +mysql> DROP DATABASE IF EXISTS db_1483421321; +mysql> CREATE DATABASE db_1483421321; +mysql> CREATE TABLE db_1483421321.t0(c0 TINYINT, handle INT NOT NULL AUTO_INCREMENT, PRIMARY KEY(handle)); +mysql> ALTER TABLE db_1483421321.t0 SET TIFLASH REPLICA 1; +func> wait_table db_1483421321 t0 +# empty table +mysql> set session tidb_isolation_read_engines='tiflash'; SELECT * FROM db_1483421321.t0 order by handle; +mysql> set session tidb_isolation_read_engines='tikv'; SELECT * FROM db_1483421321.t0 order by handle; + +mysql> ALTER TABLE db_1483421321.t0 ADD COLUMN c1 DECIMAL NULL; +mysql> ALTER TABLE db_1483421321.t0 ADD COLUMN c2 FLOAT NULL DEFAULT 0.0795439693286002; +mysql> ALTER TABLE db_1483421321.t0 ADD COLUMN c5 DECIMAL NOT NULL; +mysql> ALTER TABLE db_1483421321.t0 ADD COLUMN c4 DECIMAL DEFAULT 247262911; +mysql> ALTER TABLE db_1483421321.t0 MODIFY COLUMN c2 FLOAT NOT NULL; +mysql> ALTER TABLE db_1483421321.t0 MODIFY COLUMN c5 DECIMAL NULL; +mysql> ALTER TABLE db_1483421321.t0 MODIFY COLUMN c1 BIGINT DEFAULT -56083770 NOT NULL; + +# insert handle == 1 +mysql> INSERT INTO db_1483421321.t0 (c1, c2, c5, c4) VALUES (-2051270087, 0.44141045099028775, 0.0, 15523); +# update handle == 1 +mysql> UPDATE db_1483421321.t0 SET c5 = 870337888; + +mysql> ALTER TABLE db_1483421321.t0 MODIFY COLUMN c1 BIGINT NULL; +# insert handle == 2 +mysql> INSERT INTO db_1483421321.t0 (c2, c5, c4) VALUES (0.004406799693866592, 0.8752311290235516, 14652); +mysql> set session tidb_isolation_read_engines='tiflash'; SELECT * FROM db_1483421321.t0 order by handle; ++------+--------+-------------+--------------+-----------+-------+ +| c0 | handle | c1 | c2 | c5 | c4 | ++------+--------+-------------+--------------+-----------+-------+ +| NULL | 1 | -2051270087 | 0.44141045 | 870337888 | 15523 | +| NULL | 2 | NULL | 0.0044067996 | 1 | 14652 | ++------+--------+-------------+--------------+-----------+-------+ +mysql> set session tidb_isolation_read_engines='tikv'; SELECT * FROM db_1483421321.t0 order by handle; ++------+--------+-------------+--------------+-----------+-------+ +| c0 | handle | c1 | c2 | c5 | c4 | ++------+--------+-------------+--------------+-----------+-------+ +| NULL | 1 | -2051270087 | 0.44141045 | 870337888 | 15523 | +| NULL | 2 | NULL | 0.0044067996 | 1 | 14652 | ++------+--------+-------------+--------------+-----------+-------+ + +# insert handle == 3 +mysql> INSERT INTO db_1483421321.t0 (c2, c5, c4) VALUES (0.004406799693866592, 0.8752311290235516, 14652); +mysql> set session tidb_isolation_read_engines='tiflash'; SELECT * FROM db_1483421321.t0 order by handle; ++------+--------+-------------+--------------+-----------+-------+ +| c0 | handle | c1 | c2 | c5 | c4 | ++------+--------+-------------+--------------+-----------+-------+ +| NULL | 1 | -2051270087 | 0.44141045 | 870337888 | 15523 | +| NULL | 2 | NULL | 0.0044067996 | 1 | 14652 | +| NULL | 3 | NULL | 0.0044067996 | 1 | 14652 | ++------+--------+-------------+--------------+-----------+-------+ +mysql> set session tidb_isolation_read_engines='tikv'; SELECT * FROM db_1483421321.t0 order by handle; ++------+--------+-------------+--------------+-----------+-------+ +| c0 | handle | c1 | c2 | c5 | c4 | ++------+--------+-------------+--------------+-----------+-------+ +| NULL | 1 | -2051270087 | 0.44141045 | 870337888 | 15523 | +| NULL | 2 | NULL | 0.0044067996 | 1 | 14652 | +| NULL | 3 | NULL | 0.0044067996 | 1 | 14652 | ++------+--------+-------------+--------------+-----------+-------+ + +mysql> drop table if exists db_1483421321.t0 + +############# +# issue 10680 variation, TiFlash data inconsistent with TiKV after modifying a column from NULL TO NOT NULL then to NULL again +mysql> DROP DATABASE IF EXISTS db_1483421321; +mysql> CREATE DATABASE db_1483421321; +mysql> CREATE TABLE db_1483421321.t1(c0 TINYINT, handle INT NOT NULL AUTO_INCREMENT, PRIMARY KEY(handle)); +mysql> ALTER TABLE db_1483421321.t1 SET TIFLASH REPLICA 1; +func> wait_table db_1483421321 t1 +# empty table +mysql> set session tidb_isolation_read_engines='tiflash'; SELECT * FROM db_1483421321.t1 order by handle; +mysql> set session tidb_isolation_read_engines='tikv'; SELECT * FROM db_1483421321.t1 order by handle; + +mysql> ALTER TABLE db_1483421321.t1 ADD COLUMN c1 DECIMAL NULL; +mysql> ALTER TABLE db_1483421321.t1 ADD COLUMN c2 FLOAT NULL DEFAULT 0.0795439693286002; +mysql> ALTER TABLE db_1483421321.t1 ADD COLUMN c5 DECIMAL NOT NULL; +mysql> ALTER TABLE db_1483421321.t1 ADD COLUMN c4 DECIMAL DEFAULT 247262911; + +mysql> INSERT INTO db_1483421321.t1 (c1, c2, c5, c4) VALUES (-2051270087, 0.44141045099028775, 0.0, 15523); + +mysql> ALTER TABLE db_1483421321.t1 MODIFY COLUMN c1 BIGINT DEFAULT -56083770 NOT NULL; +mysql> UPDATE db_1483421321.t1 SET c5 = 870337888; + +mysql> ALTER TABLE db_1483421321.t1 MODIFY COLUMN c1 BIGINT NULL; +mysql> INSERT INTO db_1483421321.t1 (c2, c5, c4) VALUES (0.004406799693866592, 0.8752311290235516, 14652); +mysql> set session tidb_isolation_read_engines='tiflash'; SELECT * FROM db_1483421321.t1 order by handle; ++------+--------+-------------+--------------+-----------+-------+ +| c0 | handle | c1 | c2 | c5 | c4 | ++------+--------+-------------+--------------+-----------+-------+ +| NULL | 1 | -2051270087 | 0.44141045 | 870337888 | 15523 | +| NULL | 2 | NULL | 0.0044067996 | 1 | 14652 | ++------+--------+-------------+--------------+-----------+-------+ +mysql> set session tidb_isolation_read_engines='tikv'; SELECT * FROM db_1483421321.t1 order by handle; ++------+--------+-------------+--------------+-----------+-------+ +| c0 | handle | c1 | c2 | c5 | c4 | ++------+--------+-------------+--------------+-----------+-------+ +| NULL | 1 | -2051270087 | 0.44141045 | 870337888 | 15523 | +| NULL | 2 | NULL | 0.0044067996 | 1 | 14652 | ++------+--------+-------------+--------------+-----------+-------+ + +mysql> INSERT INTO db_1483421321.t1 (c2, c5, c4) VALUES (0.44444, 0.8888, 22222); +mysql> set session tidb_isolation_read_engines='tiflash'; SELECT * FROM db_1483421321.t1 order by handle; ++------+--------+-------------+--------------+-----------+-------+ +| c0 | handle | c1 | c2 | c5 | c4 | ++------+--------+-------------+--------------+-----------+-------+ +| NULL | 1 | -2051270087 | 0.44141045 | 870337888 | 15523 | +| NULL | 2 | NULL | 0.0044067996 | 1 | 14652 | +| NULL | 3 | NULL | 0.44444 | 1 | 22222 | ++------+--------+-------------+--------------+-----------+-------+ +mysql> set session tidb_isolation_read_engines='tikv'; SELECT * FROM db_1483421321.t1 order by handle; ++------+--------+-------------+--------------+-----------+-------+ +| c0 | handle | c1 | c2 | c5 | c4 | ++------+--------+-------------+--------------+-----------+-------+ +| NULL | 1 | -2051270087 | 0.44141045 | 870337888 | 15523 | +| NULL | 2 | NULL | 0.0044067996 | 1 | 14652 | +| NULL | 3 | NULL | 0.44444 | 1 | 22222 | ++------+--------+-------------+--------------+-----------+-------+ + +mysql> drop table if exists db_1483421321.t1 diff --git a/tests/fullstack-test2/ddl/alter_default_value.test b/tests/fullstack-test2/ddl/alter_default_value.test index 041bc043ab0..792f75906c5 100644 --- a/tests/fullstack-test2/ddl/alter_default_value.test +++ b/tests/fullstack-test2/ddl/alter_default_value.test @@ -131,63 +131,3 @@ mysql> set session tidb_isolation_read_engines='tiflash'; select col1, col2, col +------+------+------+------+------+------+---------------------+---------------------+------+--------+--------+---------------------+----------+-------+------------+ mysql> drop table if exists test.t -############# -# issue 10680, TiFlash data inconsistent with TiKV after modifying a column from NOT NULL to NULL -mysql> DROP DATABASE IF EXISTS db_1483421321; -mysql> CREATE DATABASE db_1483421321; -mysql> CREATE TABLE db_1483421321.t0(c0 TINYINT, handle INT NOT NULL AUTO_INCREMENT, PRIMARY KEY(handle)); -mysql> ALTER TABLE db_1483421321.t0 SET TIFLASH REPLICA 1; -func> wait_table db_1483421321 t0 -# empty table -mysql> set session tidb_isolation_read_engines='tiflash'; SELECT * FROM db_1483421321.t0 order by handle; -mysql> set session tidb_isolation_read_engines='tikv'; SELECT * FROM db_1483421321.t0 order by handle; - -mysql> ALTER TABLE db_1483421321.t0 ADD COLUMN c1 DECIMAL NULL; -mysql> ALTER TABLE db_1483421321.t0 ADD COLUMN c2 FLOAT NULL DEFAULT 0.0795439693286002; -mysql> ALTER TABLE db_1483421321.t0 ADD COLUMN c5 DECIMAL NOT NULL; -mysql> ALTER TABLE db_1483421321.t0 ADD COLUMN c4 DECIMAL DEFAULT 247262911; -mysql> ALTER TABLE db_1483421321.t0 MODIFY COLUMN c2 FLOAT NOT NULL; -mysql> ALTER TABLE db_1483421321.t0 MODIFY COLUMN c5 DECIMAL NULL; -mysql> ALTER TABLE db_1483421321.t0 MODIFY COLUMN c1 BIGINT DEFAULT -56083770 NOT NULL; - -# first insert -mysql> INSERT IGNORE INTO db_1483421321.t0 (c1, c2, c5, c4) VALUES (-2051270087, 0.44141045099028775, 0.0, 15523); -# update and generate a new raftlog -mysql> UPDATE db_1483421321.t0 SET c5 = 870337888; -mysql> ALTER TABLE db_1483421321.t0 MODIFY COLUMN c1 BIGINT NULL; - -# insert and generate the third raftlog -mysql> INSERT INTO db_1483421321.t0 (c2, c5, c4) VALUES (0.004406799693866592, 0.8752311290235516, 14652); -mysql> set session tidb_isolation_read_engines='tiflash'; SELECT * FROM db_1483421321.t0 order by handle; -+------+--------+-------------+--------------+-----------+-------+ -| c0 | handle | c1 | c2 | c5 | c4 | -+------+--------+-------------+--------------+-----------+-------+ -| NULL | 1 | -2051270087 | 0.44141045 | 870337888 | 15523 | -| NULL | 2 | NULL | 0.0044067996 | 1 | 14652 | -+------+--------+-------------+--------------+-----------+-------+ -mysql> set session tidb_isolation_read_engines='tikv'; SELECT * FROM db_1483421321.t0 order by handle; -+------+--------+-------------+--------------+-----------+-------+ -| c0 | handle | c1 | c2 | c5 | c4 | -+------+--------+-------------+--------------+-----------+-------+ -| NULL | 1 | -2051270087 | 0.44141045 | 870337888 | 15523 | -| NULL | 2 | NULL | 0.0044067996 | 1 | 14652 | -+------+--------+-------------+--------------+-----------+-------+ - -# more insert -mysql> INSERT INTO db_1483421321.t0 (c2, c5, c4) VALUES (0.004406799693866592, 0.8752311290235516, 14652); -mysql> set session tidb_isolation_read_engines='tiflash'; SELECT * FROM db_1483421321.t0 order by handle; -+------+--------+-------------+--------------+-----------+-------+ -| c0 | handle | c1 | c2 | c5 | c4 | -+------+--------+-------------+--------------+-----------+-------+ -| NULL | 1 | -2051270087 | 0.44141045 | 870337888 | 15523 | -| NULL | 2 | NULL | 0.0044067996 | 1 | 14652 | -| NULL | 3 | NULL | 0.0044067996 | 1 | 14652 | -+------+--------+-------------+--------------+-----------+-------+ -mysql> set session tidb_isolation_read_engines='tikv'; SELECT * FROM db_1483421321.t0 order by handle; -+------+--------+-------------+--------------+-----------+-------+ -| c0 | handle | c1 | c2 | c5 | c4 | -+------+--------+-------------+--------------+-----------+-------+ -| NULL | 1 | -2051270087 | 0.44141045 | 870337888 | 15523 | -| NULL | 2 | NULL | 0.0044067996 | 1 | 14652 | -| NULL | 3 | NULL | 0.0044067996 | 1 | 14652 | -+------+--------+-------------+--------------+-----------+-------+ From a428cd65c13b4c8c04920173bbe3b50ecec72e60 Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Wed, 28 Jan 2026 21:53:03 +0800 Subject: [PATCH 12/14] Revert "Add debug log and test case" This reverts commit d8448b5cf013e3c43d0561ddc45278c2edf049d6. --- .../KVStore/Decode/PartitionStreams.cpp | 12 ------ .../KVStore/Decode/RegionBlockReader.cpp | 10 ----- .../KVStore/MultiRaft/RaftCommands.cpp | 1 - dbms/src/Storages/StorageDeltaMerge.cpp | 8 ---- dbms/src/TiDB/Decode/RowCodec.cpp | 37 ------------------- 5 files changed, 68 deletions(-) diff --git a/dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp b/dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp index 31f445fbe1f..4e47246b305 100644 --- a/dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp +++ b/dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp @@ -159,18 +159,6 @@ static inline bool atomicReadWrite( std::tie(decoding_schema_snapshot, block_ptr) = storage->getSchemaSnapshotAndBlockForDecoding(lock, true, should_handle_version_col); block_decoding_schema_epoch = decoding_schema_snapshot->decoding_schema_epoch; - const auto & table_info = storage->getTableInfo(); - LOG_INFO( - Logger::get("dddddddddd"), - "decode with schema snapshot, keyspace={} table_id={} region_id={} epoch={} update_ts={} " - "with_version_column={} columns={}", - rw_ctx.keyspace_id, - rw_ctx.table_id, - region->id(), - block_decoding_schema_epoch, - table_info.update_timestamp, - should_handle_version_col, - table_info.columns.size()); auto reader = RegionBlockReader(decoding_schema_snapshot); if (!reader.read(*block_ptr, data_list_read, force_decode)) diff --git a/dbms/src/Storages/KVStore/Decode/RegionBlockReader.cpp b/dbms/src/Storages/KVStore/Decode/RegionBlockReader.cpp index 9a1696abaa0..02706d6ac17 100644 --- a/dbms/src/Storages/KVStore/Decode/RegionBlockReader.cpp +++ b/dbms/src/Storages/KVStore/Decode/RegionBlockReader.cpp @@ -44,11 +44,6 @@ bool RegionBlockReader::read(Block & block, const ReadList & data_list, bool for { try { - for (const auto & data : data_list) - { - LOG_INFO(Logger::get("dddddddddd"), "RegionBlockReader::read data.value={}", data.value->toDebugString()); - } - switch (schema_snapshot->pk_type) { case TMTPKType::INT64: @@ -182,11 +177,6 @@ struct VersionColResolver template bool RegionBlockReader::readImpl(Block & block, const ReadList & data_list, bool force_decode) { - LOG_INFO( - Logger::get("dddddddddd"), - "RegionBlockReader::readImpl start, schema_snapshot.column_defines={}", - *schema_snapshot->column_defines); - VersionColResolver version_col_resolver; version_col_resolver.check(block, schema_snapshot->column_defines->size()); // The column_ids to read according to schema_snapshot, each elem is (column_id, block_pos) diff --git a/dbms/src/Storages/KVStore/MultiRaft/RaftCommands.cpp b/dbms/src/Storages/KVStore/MultiRaft/RaftCommands.cpp index e2e05ce25f5..bfb9d2b06e7 100644 --- a/dbms/src/Storages/KVStore/MultiRaft/RaftCommands.cpp +++ b/dbms/src/Storages/KVStore/MultiRaft/RaftCommands.cpp @@ -368,7 +368,6 @@ std::pair Region::handleWriteRaftCmd( { auto tikv_key = TiKVKey(cmds.keys[i].data, cmds.keys[i].len); auto tikv_value = TiKVValue(cmds.vals[i].data, cmds.vals[i].len); - LOG_INFO(Logger::get("dddddddddd"), "Region::handleWriteRaftCmd Put key={}, value={}", tikv_key.toDebugString(), tikv_value.toDebugString()); if (cf == ColumnFamilyType::Write) { write_put_key_count++; diff --git a/dbms/src/Storages/StorageDeltaMerge.cpp b/dbms/src/Storages/StorageDeltaMerge.cpp index 7ac3f543642..14926b90e09 100644 --- a/dbms/src/Storages/StorageDeltaMerge.cpp +++ b/dbms/src/Storages/StorageDeltaMerge.cpp @@ -1254,14 +1254,6 @@ std::pair StorageDeltaMerg store->getHandle(), decoding_schema_epoch++, with_version_column); - LOG_INFO( - Logger::get("dddddddddd"), - "Refresh decoding schema snapshot, table_id={} epoch={} update_ts={} with_version_column={} table_info={}", - tidb_table_info.id, - decoding_schema_snapshot->decoding_schema_epoch, - tidb_table_info.update_timestamp, - with_version_column, - tidb_table_info.serialize()); cache_blocks.clear(); decoding_schema_changed = false; } diff --git a/dbms/src/TiDB/Decode/RowCodec.cpp b/dbms/src/TiDB/Decode/RowCodec.cpp index 6ce5b376c68..3c5eec071ec 100644 --- a/dbms/src/TiDB/Decode/RowCodec.cpp +++ b/dbms/src/TiDB/Decode/RowCodec.cpp @@ -13,7 +13,6 @@ // limitations under the License. #include -#include #include #include #include @@ -486,12 +485,6 @@ bool appendRowV2ToBlockImpl( raw_value, num_not_null_columns, value_offsets); - LOG_INFO( - Logger::get("dddddddddd"), - "not_null_column_ids={} null_column_ids={}", - not_null_column_ids, - null_column_ids); - size_t values_start_pos = cursor; // how many not null columns have been processed size_t idx_not_null = 0; @@ -514,12 +507,6 @@ bool appendRowV2ToBlockImpl( auto next_datum_column_id = is_null ? null_column_ids[idx_null] : not_null_column_ids[idx_not_null]; const auto next_column_id = column_ids_iter->first; - LOG_INFO( - Logger::get("dddddddddd"), - "next_column_id={} next_datum_column_id={} force_decode={}", - next_column_id, - next_datum_column_id, - force_decode); if (next_column_id > next_datum_column_id) { // The next_column_id to read is bigger than the next_datum_column_id in encoded row. @@ -541,19 +528,6 @@ bool appendRowV2ToBlockImpl( // a column. // Fill with default value and continue to read data for next column id. const auto & column_info = column_infos[column_ids_iter->second]; - LOG_INFO( - Logger::get("dddddddddd"), - "appendRowV2ToBlockImpl: fill default value for missing column," - " next_column_id={} next_datum_column_id={} block_column_pos={}" - " column_info={{name={} id={} not_null={} no_default_val={} default_value={}}}", - next_column_id, - next_datum_column_id, - block_column_pos, - column_info.name, - column_info.id, - column_info.hasNotNullFlag(), - column_info.hasNoDefaultValueFlag(), - applyVisitor(FieldVisitorToString(), column_info.defaultValueToField())); if (!addDefaultValueToColumnIfPossible( column_info, block, @@ -637,17 +611,6 @@ bool appendRowV2ToBlockImpl( if (column_ids_iter->first != pk_handle_id) { const auto & column_info = column_infos[column_ids_iter->second]; - LOG_INFO( - Logger::get("dddddddddd"), - "appendRowV2ToBlockImpl: fill default value for missing column," - " block_column_pos={}" - " column_info={{name={} id={} not_null={} no_default_val={} default_value={}}}", - block_column_pos, - column_info.name, - column_info.id, - column_info.hasNotNullFlag(), - column_info.hasNoDefaultValueFlag(), - applyVisitor(FieldVisitorToString(), column_info.defaultValueToField())); if (!addDefaultValueToColumnIfPossible( column_info, block, From 0499e1cb349a009641a58eebc72f566dd3b7dfe9 Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Wed, 28 Jan 2026 23:39:39 +0800 Subject: [PATCH 13/14] Add and fix unit tests Signed-off-by: JaySon-Huang --- .../tests/gtest_region_block_reader.cpp | 32 +++++++++++++++++++ dbms/src/TiDB/tests/RowCodecTestUtils.h | 1 + 2 files changed, 33 insertions(+) diff --git a/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp b/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp index 42c299c83ff..c059f3e0d31 100644 --- a/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp +++ b/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp @@ -710,6 +710,11 @@ try R"({"cols":[{"id":1,"name":{"L":"c0","O":"c0"},"offset":0,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":4,"Tp":1}},{"id":2,"name":{"L":"handle","O":"handle"},"offset":1,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":515,"Flen":11,"Tp":3}},{"origin_default":"-56083770","id":7,"name":{"L":"c1","O":"c1"},"offset":2,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":1,"Flen":20,"Tp":8}},{"id":4,"name":{"L":"c2","O":"c2"},"offset":3,"origin_default":"0.07954397","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":-1,"Flag":4097,"Flen":12,"Tp":4}},{"id":5,"name":{"L":"c5","O":"c5"},"offset":4,"origin_default":"0","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}},{"default":"247262911","id":6,"name":{"L":"c4","O":"c4"},"offset":5,"origin_default":"247262911","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}}],"id":636,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"t0","O":"t0"},"pk_is_handle":true,"state":5,"tiflash_replica":{"Available":true,"Count":1},"update_timestamp":463845180343844895})", NullspaceID); + // With this table_info, column "c1" has the "NOT NULL" flag and has origin default "-56083770", but the state is not public + TableInfo table_info_c1_not_null_with_origin_default_non_public( + R"({"cols":[{"id":1,"name":{"L":"c0","O":"c0"},"offset":0,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":4,"Tp":1}},{"id":2,"name":{"L":"handle","O":"handle"},"offset":1,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":515,"Flen":11,"Tp":3}},{"origin_default":"-56083770","id":7,"name":{"L":"c1","O":"c1"},"offset":2,"state":3,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":1,"Flen":20,"Tp":8}},{"id":4,"name":{"L":"c2","O":"c2"},"offset":3,"origin_default":"0.07954397","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":-1,"Flag":4097,"Flen":12,"Tp":4}},{"id":5,"name":{"L":"c5","O":"c5"},"offset":4,"origin_default":"0","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}},{"default":"247262911","id":6,"name":{"L":"c4","O":"c4"},"offset":5,"origin_default":"247262911","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}}],"id":636,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"t0","O":"t0"},"pk_is_handle":true,"state":5,"tiflash_replica":{"Available":true,"Count":1},"update_timestamp":463845180343844895})", + NullspaceID); + // With this table_info, column "c1" does not have the "NOT NULL" flag TableInfo table_info_c1_nullable( R"({"cols":[{"id":1,"name":{"L":"c0","O":"c0"},"offset":0,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":4,"Tp":1}},{"id":2,"name":{"L":"handle","O":"handle"},"offset":1,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":515,"Flen":11,"Tp":3}},{"id":7,"name":{"L":"c1","O":"c1"},"offset":2,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":20,"Tp":8}},{"id":4,"name":{"L":"c2","O":"c2"},"offset":3,"origin_default":"0.07954397","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":-1,"Flag":4097,"Flen":12,"Tp":4}},{"id":5,"name":{"L":"c5","O":"c5"},"offset":4,"origin_default":"0","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}},{"default":"247262911","id":6,"name":{"L":"c4","O":"c4"},"offset":5,"origin_default":"247262911","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}}],"id":636,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"t0","O":"t0"},"pk_is_handle":true,"state":5,"tiflash_replica":{"Available":true,"Count":1},"update_timestamp":463844343842340870})", @@ -811,6 +816,33 @@ try // verify the default value is filled correctly ASSERT_COLUMN_EQ( // res_block.getByName("c1"), + // the thrid elem is filled wih origin_default + createColumn({-2051270087, -2051270087, -56083770})); + } + + // Test with `table_info_c1_not_null_with_origin_default_non_public` + decoding_schema = getDecodingStorageSchemaSnapshot(table_info_c1_not_null_with_origin_default_non_public); + { + // force_decode=false can not decode because there are + // missing value for column with not null flag and the state is not public + auto reader = RegionBlockReader(decoding_schema); + Block res_block = createBlockSortByColumnID(decoding_schema); + ASSERT_FALSE(reader.read(res_block, *data_list_read, false)); + } + { + // force_decode=true can decode the block, and filling the default value for c1 + auto reader = RegionBlockReader(decoding_schema); + Block res_block = createBlockSortByColumnID(decoding_schema); + ASSERT_TRUE(reader.read(res_block, *data_list_read, true)); + LOG_INFO( + Logger::get(), + "Decoded block:\n{}", + DB::tests::getColumnsContent(res_block.getColumnsWithTypeAndName())); + ASSERT_EQ(res_block.getByName("c1").type->getName(), "Int64"); + // verify the default value is filled correctly + ASSERT_COLUMN_EQ( // + res_block.getByName("c1"), + // the thrid elem is filled wih origin_default createColumn({-2051270087, -2051270087, -56083770})); } diff --git a/dbms/src/TiDB/tests/RowCodecTestUtils.h b/dbms/src/TiDB/tests/RowCodecTestUtils.h index 6b27e51878d..0d2354b5edd 100644 --- a/dbms/src/TiDB/tests/RowCodecTestUtils.h +++ b/dbms/src/TiDB/tests/RowCodecTestUtils.h @@ -123,6 +123,7 @@ ColumnInfo getColumnInfo(ColumnID id) column_info.setUnsignedFlag(); if constexpr (!nullable) column_info.setNotNullFlag(); + column_info.state = TiDB::SchemaState::StatePublic; return column_info; } From f264ae94b89d636bb46c021f95e504e993c325f4 Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Thu, 29 Jan 2026 01:06:17 +0800 Subject: [PATCH 14/14] minor polish Signed-off-by: JaySon-Huang --- dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp | 2 +- tests/fullstack-test2/ddl/alter_default_value.test | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp b/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp index c059f3e0d31..eeb2ee226e0 100644 --- a/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp +++ b/dbms/src/Storages/KVStore/tests/gtest_region_block_reader.cpp @@ -702,7 +702,7 @@ try // With this table_info, column "c1" is "NOT NULL" and has no default value TableInfo table_info_c1_not_null_no_default_value( - R"({"cols":[{"id":1,"name":{"L":"c0","O":"c0"},"offset":0,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":4,"Tp":1}},{"id":2,"name":{"L":"handle","O":"handle"},"offset":1,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":515,"Flen":11,"Tp":3}},{"default":"-56083770","id":7,"name":{"L":"c1","O":"c1"},"offset":2,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":4097,"Flen":20,"Tp":8}},{"id":4,"name":{"L":"c2","O":"c2"},"offset":3,"origin_default":"0.07954397","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":-1,"Flag":4097,"Flen":12,"Tp":4}},{"id":5,"name":{"L":"c5","O":"c5"},"offset":4,"origin_default":"0","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}},{"default":"247262911","id":6,"name":{"L":"c4","O":"c4"},"offset":5,"origin_default":"247262911","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}}],"id":636,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"t0","O":"t0"},"pk_is_handle":true,"state":5,"tiflash_replica":{"Available":true,"Count":1},"update_timestamp":463845180343844895})", + R"({"cols":[{"id":1,"name":{"L":"c0","O":"c0"},"offset":0,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":4,"Tp":1}},{"id":2,"name":{"L":"handle","O":"handle"},"offset":1,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":515,"Flen":11,"Tp":3}},{"id":7,"name":{"L":"c1","O":"c1"},"offset":2,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":4097,"Flen":20,"Tp":8}},{"id":4,"name":{"L":"c2","O":"c2"},"offset":3,"origin_default":"0.07954397","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":-1,"Flag":4097,"Flen":12,"Tp":4}},{"id":5,"name":{"L":"c5","O":"c5"},"offset":4,"origin_default":"0","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}},{"default":"247262911","id":6,"name":{"L":"c4","O":"c4"},"offset":5,"origin_default":"247262911","state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":10,"Tp":246}}],"id":636,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"t0","O":"t0"},"pk_is_handle":true,"state":5,"tiflash_replica":{"Available":true,"Count":1},"update_timestamp":463845180343844895})", NullspaceID); // With this table_info, column "c1" has the "NOT NULL" flag and has origin default "-56083770" diff --git a/tests/fullstack-test2/ddl/alter_default_value.test b/tests/fullstack-test2/ddl/alter_default_value.test index 792f75906c5..a0385362f2b 100644 --- a/tests/fullstack-test2/ddl/alter_default_value.test +++ b/tests/fullstack-test2/ddl/alter_default_value.test @@ -130,4 +130,3 @@ mysql> set session tidb_isolation_read_engines='tiflash'; select col1, col2, col | 30 | 0000 | 0.00 | 1.23 | 123 | sss | 2017-02-11 00:00:00 | 2018-11-04 23:12:03 | 2012 | value3 | value1 | 2018-11-04 23:12:03 | 12:18:30 | NULL | 10203 | +------+------+------+------+------+------+---------------------+---------------------+------+--------+--------+---------------------+----------+-------+------------+ mysql> drop table if exists test.t -