ddl: fix TypeBit default value (#9467) (#9478)

close #9461 ddl: fix TypeBit default value Signed-off-by: JaySon-Huang <[email protected]> Co-authored-by: JaySon-Huang <[email protected]>
pingcap · Sep 27, 2024 · 9fd7ba2 · 9fd7ba2
1 parent 2faf287
commit 9fd7ba2
Show file tree

Hide file tree

Showing 6 changed files with 111 additions and 36 deletions.
diff --git a/contrib/poco b/contrib/poco
diff --git a/dbms/src/TiDB/Schema/TiDB.cpp b/dbms/src/TiDB/Schema/TiDB.cpp
@@ -152,14 +152,7 @@ Field ColumnInfo::defaultValueToField() const
         });
     case TypeBit:
     {
-        // TODO: We shall use something like `orig_default_bit`, which will never change once created,
-        //  rather than `default_bit`, which could be altered.
-        //  See https://github.com/pingcap/tidb/issues/17641 and https://github.com/pingcap/tidb/issues/17642
-        const auto & bit_value = default_bit_value;
-        // TODO: There might be cases that `orig_default` is not null but `default_bit` is null,
-        //  i.e. bit column added with an default value but later modified to another.
-        //  For these cases, neither `orig_default` (may get corrupted) nor `default_bit` (modified) is correct.
-        //  This is a bug anyway, we choose to make it simple, i.e. use `default_bit`.
+        const auto & bit_value = origin_default_bit_value;
         if (bit_value.isEmpty())
         {
             if (hasNotNullFlag())
@@ -378,6 +371,8 @@ try
     json->set("origin_default", origin_default_value);
     json->set("default", default_value);
     json->set("default_bit", default_bit_value);
+    if (!origin_default_bit_value.isEmpty())
+        json->set("origin_default_bit", origin_default_bit_value);
     Poco::JSON::Object::Ptr tp_json = new Poco::JSON::Object();
     tp_json->set("Tp", static_cast<Int32>(tp));
     tp_json->set("Flag", flag);
@@ -432,6 +427,8 @@ try
     flag = type_json->getValue<UInt32>("Flag");
     flen = type_json->getValue<Int64>("Flen");
     decimal = type_json->getValue<Int64>("Decimal");
+    if (!json->isNull("origin_default_bit"))
+        origin_default_bit_value = json->get("origin_default_bit");
     if (!type_json->isNull("Elems"))
     {
         auto elems_arr = type_json->getArray("Elems");
@@ -1267,7 +1264,11 @@ ColumnInfo toTiDBColumnInfo(const tipb::ColumnInfo & tipb_column_info)
     // TiFlash get default value from origin_default_value, check `Field ColumnInfo::defaultValueToField() const`
     // So we need to set origin_default_value to tipb_column_info.default_val()
     // Related logic in tidb, https://github.com/pingcap/tidb/blob/45318da24d8e4c0c6aab836d291a33f949dd18bf/pkg/table/tables/tables.go#L2303-L2329
-    tidb_column_info.origin_default_value = tipb_column_info.default_val();
+    // For TypeBit, we need to set origin_default_bit_value to tipb_column_info.default_val().
+    if (tidb_column_info.tp == TypeBit)
+        tidb_column_info.origin_default_bit_value = tipb_column_info.default_val();
+    else
+        tidb_column_info.origin_default_value = tipb_column_info.default_val();
     tidb_column_info.collate = tipb_column_info.collation();
     for (int i = 0; i < tipb_column_info.elems_size(); ++i)
         tidb_column_info.elems.emplace_back(tipb_column_info.elems(i), i + 1);

diff --git a/dbms/src/TiDB/Schema/TiDB.h b/dbms/src/TiDB/Schema/TiDB.h
@@ -186,6 +186,7 @@ struct ColumnInfo
     Int32 offset = -1;
     Poco::Dynamic::Var origin_default_value;
     Poco::Dynamic::Var default_value;
+    Poco::Dynamic::Var origin_default_bit_value;
     Poco::Dynamic::Var default_bit_value;
     TP tp = TypeDecimal; // TypeDecimal is not used by TiDB.
     UInt32 flag = 0;

diff --git a/dbms/src/TiDB/tests/gtest_table_info.cpp b/dbms/src/TiDB/tests/gtest_table_info.cpp
@@ -18,6 +18,7 @@
 #include <Parsers/ParserCreateQuery.h>
 #include <Parsers/parseQuery.h>
 #include <Poco/Logger.h>
+#include <Storages/KVStore/StorageEngineType.h>
 #include <Storages/registerStorages.h>
 #include <TestUtils/TiFlashTestBasic.h>
 #include <TiDB/Decode/TypeMapping.h>
@@ -150,24 +151,20 @@ struct StmtCase
         EXPECT_EQ(json1, json2) << "Table info unescaped serde mismatch:\n" + json1 + "\n" + json2;
 
         // generate create statement with db_info and table_info
-        auto verify_stmt = [&](TiDB::StorageEngine engine_type) {
-            table_info.engine_type = engine_type;
-            String stmt = createTableStmt(
-                db_info.keyspace_id,
-                db_info.id,
-                table_info,
-                MockSchemaNameMapper(),
-                tombstone,
-                Logger::get());
-            EXPECT_EQ(stmt, create_stmt_dm) << "Table info create statement mismatch:\n" + stmt + "\n" + create_stmt_dm;
-
-            json1 = extractTableInfoFromCreateStatement(stmt, table_info.name);
-            table_info.deserialize(json1);
-            json2 = table_info.serialize();
-            EXPECT_EQ(json1, json2) << "Table info escaped serde mismatch:\n" + json1 + "\n" + json2;
-        };
-
-        verify_stmt(TiDB::StorageEngine::DT);
+        table_info.engine_type = TiDB::StorageEngine::DT;
+        String stmt = createTableStmt(
+            db_info.keyspace_id,
+            db_info.id,
+            table_info,
+            MockSchemaNameMapper(),
+            tombstone,
+            Logger::get());
+        EXPECT_EQ(stmt, create_stmt_dm) << "Table info create statement mismatch:\n" + stmt + "\n" + create_stmt_dm;
+
+        json1 = extractTableInfoFromCreateStatement(stmt, table_info.name);
+        table_info.deserialize(json1);
+        json2 = table_info.serialize();
+        EXPECT_EQ(json1, json2) << "Table info escaped serde mismatch:\n" + json1 + "\n" + json2;
     }
 
 private:
@@ -185,20 +182,21 @@ struct StmtCase
 TEST(TiDBTableInfoTest, GenCreateTableStatement)
 try
 {
+    // clang-format off
     auto cases = {
         StmtCase{
             1145, //
             0,
             R"json({"id":1939,"db_name":{"O":"customer","L":"customer"},"charset":"utf8mb4","collate":"utf8mb4_bin","state":5})json", //
             R"json({"id":1145,"name":{"O":"customerdebt","L":"customerdebt"},"cols":[{"id":1,"name":{"O":"id","L":"id"},"offset":0,"origin_default":null,"default":null,"default_bit":null,"type":{"Tp":8,"Flag":515,"Flen":20,"Decimal":0},"state":5,"comment":"i\"d"}],"state":5,"pk_is_handle":true,"schema_version":-1,"comment":"负债信息","partition":null})json", //
-            R"stmt(CREATE TABLE `db_1939`.`t_1145`(`id` Int64) Engine = DeltaMerge((`id`), '{"cols":[{"comment":"i\\"d","default":null,"default_bit":null,"id":1,"name":{"L":"id","O":"id"},"offset":0,"origin_default":null,"state":5,"type":{"Charset":null,"Collate":null,"Decimal":0,"Elems":null,"Flag":515,"Flen":20,"Tp":8}}],"comment":"\\u8D1F\\u503A\\u4FE1\\u606F","id":1145,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"customerdebt","O":"customerdebt"},"partition":null,"pk_is_handle":true,"schema_version":-1,"state":5,"tiflash_replica":{"Count":0},"update_timestamp":0}', 0))stmt", //
+            R"stmt(CREATE TABLE `db_1939`.`t_1145`(`id` Int64) Engine = DeltaMerge((`id`), '{"cols":[{"comment":"i\\"d","default":null,"default_bit":null,"id":1,"name":{"L":"id","O":"id"},"offset":0,"origin_default":null,"state":5,"type":{"Charset":null,"Collate":null,"Decimal":0,"Elems":null,"Flag":515,"Flen":20,"Tp":8}}],"comment":"负债信息","id":1145,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"customerdebt","O":"customerdebt"},"partition":null,"pk_is_handle":true,"schema_version":-1,"state":5,"tiflash_replica":{"Count":0},"update_timestamp":0}', 0))stmt", //
         },
         StmtCase{
             2049, //
             0,
             R"json({"id":1939,"db_name":{"O":"customer","L":"customer"},"charset":"utf8mb4","collate":"utf8mb4_bin","state":5})json", //
             R"json({"id":2049,"name":{"O":"customerdebt","L":"customerdebt"},"cols":[{"id":1,"name":{"O":"id","L":"id"},"offset":0,"origin_default":null,"default":null,"default_bit":null,"type":{"Tp":8,"Flag":515,"Flen":20,"Decimal":0},"state":5,"comment":"i\"d"}],"state":5,"pk_is_handle":true,"schema_version":-1,"comment":"负债信息","update_timestamp":404545295996944390,"partition":null})json", //
-            R"stmt(CREATE TABLE `db_1939`.`t_2049`(`id` Int64) Engine = DeltaMerge((`id`), '{"cols":[{"comment":"i\\"d","default":null,"default_bit":null,"id":1,"name":{"L":"id","O":"id"},"offset":0,"origin_default":null,"state":5,"type":{"Charset":null,"Collate":null,"Decimal":0,"Elems":null,"Flag":515,"Flen":20,"Tp":8}}],"comment":"\\u8D1F\\u503A\\u4FE1\\u606F","id":2049,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"customerdebt","O":"customerdebt"},"partition":null,"pk_is_handle":true,"schema_version":-1,"state":5,"tiflash_replica":{"Count":0},"update_timestamp":404545295996944390}', 0))stmt", //
+            R"stmt(CREATE TABLE `db_1939`.`t_2049`(`id` Int64) Engine = DeltaMerge((`id`), '{"cols":[{"comment":"i\\"d","default":null,"default_bit":null,"id":1,"name":{"L":"id","O":"id"},"offset":0,"origin_default":null,"state":5,"type":{"Charset":null,"Collate":null,"Decimal":0,"Elems":null,"Flag":515,"Flen":20,"Tp":8}}],"comment":"负债信息","id":2049,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"customerdebt","O":"customerdebt"},"partition":null,"pk_is_handle":true,"schema_version":-1,"state":5,"tiflash_replica":{"Count":0},"update_timestamp":404545295996944390}', 0))stmt", //
         },
         StmtCase{
             31, //
@@ -242,7 +240,15 @@ try
             R"json({"id":31,"name":{"O":"range_part_t","L":"range_part_t"},"charset":"utf8mb4","collate":"utf8mb4_bin","cols":[{"id":1,"name":{"O":"i","L":"i"},"offset":0,"origin_default":null,"default":null,"default_bit":null,"generated_expr_string":"","generated_stored":false,"dependences":null,"type":{"Tp":3,"Flag":0,"Flen":11,"Decimal":0,"Charset":"binary","Collate":"binary","Elems":null},"state":5,"comment":"","version":0}],"index_info":null,"fk_info":null,"state":5,"pk_is_handle":false,"schema_version":-1,"comment":"","auto_inc_id":0,"max_col_id":1,"max_idx_id":0,"update_timestamp":407445773801488390,"ShardRowIDBits":0,"partition":{"type":1,"expr":"`i`","columns":null,"enable":true,"definitions":[{"id":32,"name":{"O":"p0","L":"p0"},"less_than":["0"]},{"id":33,"name":{"O":"p1","L":"p1"},"less_than":["100"]}],"num":0},"compression":"","version":1})json", //
             R"stmt(CREATE TABLE `db_1`.`t_32`(`i` Nullable(Int32), `_tidb_rowid` Int64) Engine = DeltaMerge((`_tidb_rowid`), '{"belonging_table_id":31,"cols":[{"comment":"","default":null,"default_bit":null,"id":1,"name":{"L":"i","O":"i"},"offset":0,"origin_default":null,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Elems":null,"Flag":0,"Flen":11,"Tp":3}}],"comment":"","id":32,"index_info":[],"is_common_handle":false,"is_partition_sub_table":true,"keyspace_id":4294967295,"name":{"L":"range_part_t_32","O":"range_part_t_32"},"partition":null,"pk_is_handle":false,"schema_version":-1,"state":5,"tiflash_replica":{"Count":0},"update_timestamp":407445773801488390}', 1700815239))stmt", //
         },
+        StmtCase{
+            546, //
+            0,
+            R"json({"id":2,"db_name":{"O":"test","L":"test"},"charset":"utf8mb4","collate":"utf8mb4_bin","state":5})json", //
+            R"json({"id":546,"name":{"O":"tcfc7825f","L":"tcfc7825f"},"charset":"utf8mb4","collate":"utf8mb4_general_ci","cols":[{"id":1,"name":{"O":"col_86","L":"col_86"},"offset":0,"origin_default":null,"default":null,"default_bit":null,"default_is_expr":false,"generated_expr_string":"","generated_stored":false,"dependences":null,"type":{"Tp":252,"Flag":128,"Flen":65535,"Decimal":0,"Charset":"binary","Collate":"binary","Elems":null,"ElemsIsBinaryLit":null,"Array":false},"state":5,"comment":"","hidden":false,"change_state_info":null,"version":2},{"id":2,"name":{"O":"col_87","L":"col_87"},"offset":1,"origin_default":null,"default":"1994-05-0600:00:00","default_bit":null,"default_is_expr":false,"generated_expr_string":"","generated_stored":false,"dependences":null,"type":{"Tp":12,"Flag":129,"Flen":19,"Decimal":0,"Charset":"binary","Collate":"binary","Elems":null,"ElemsIsBinaryLit":null,"Array":false},"state":5,"comment":"","hidden":false,"change_state_info":null,"version":2},{"id":3,"name":{"O":"col_88","L":"col_88"},"offset":2,"origin_default":null,"default":null,"default_bit":null,"default_is_expr":false,"generated_expr_string":"","generated_stored":false,"dependences":null,"type":{"Tp":16,"Flag":32,"Flen":42,"Decimal":0,"Charset":"binary","Collate":"binary","Elems":null,"ElemsIsBinaryLit":null,"Array":false},"state":5,"comment":"","hidden":false,"change_state_info":null,"version":2},{"id":4,"name":{"O":"col_89","L":"col_89"},"offset":3,"origin_default":null,"default":"\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000","default_bit":null,"default_is_expr":false,"generated_expr_string":"","generated_stored":false,"dependences":null,"type":{"Tp":254,"Flag":129,"Flen":21,"Decimal":0,"Charset":"binary","Collate":"binary","Elems":null,"ElemsIsBinaryLit":null,"Array":false},"state":5,"comment":"","hidden":false,"change_state_info":null,"version":2},{"id":5,"name":{"O":"col_90","L":"col_90"},"offset":4,"origin_default":null,"default":null,"default_bit":null,"default_is_expr":false,"generated_expr_string":"","generated_stored":false,"dependences":null,"type":{"Tp":1,"Flag":4129,"Flen":3,"Decimal":0,"Charset":"binary","Collate":"binary","Elems":null,"ElemsIsBinaryLit":null,"Array":false},"state":5,"comment":"","hidden":false,"change_state_info":null,"version":2},{"id":6,"name":{"O":"col_91","L":"col_91"},"offset":5,"origin_default":null,"default":"\u0007\u0007","default_bit":"Bwc=","default_is_expr":false,"generated_expr_string":"","generated_stored":false,"dependences":null,"type":{"Tp":16,"Flag":32,"Flen":12,"Decimal":0,"Charset":"binary","Collate":"binary","Elems":null,"ElemsIsBinaryLit":null,"Array":false},"state":5,"comment":"","hidden":false,"change_state_info":null,"version":2},{"id":7,"name":{"O":"col_92","L":"col_92"},"offset":6,"origin_default":null,"default":"kY~6to6H4ut*QAPrj@\u0026","default_bit":null,"default_is_expr":false,"generated_expr_string":"","generated_stored":false,"dependences":null,"type":{"Tp":15,"Flag":129,"Flen":343,"Decimal":0,"Charset":"binary","Collate":"binary","Elems":null,"ElemsIsBinaryLit":null,"Array":false},"state":5,"comment":"","hidden":false,"change_state_info":null,"version":2},{"id":8,"name":{"O":"col_93","L":"col_93"},"offset":7,"origin_default":null,"default":null,"default_bit":null,"default_is_expr":false,"generated_expr_string":"","generated_stored":false,"dependences":null,"type":{"Tp":245,"Flag":128,"Flen":4294967295,"Decimal":0,"Charset":"binary","Collate":"binary","Elems":null,"ElemsIsBinaryLit":null,"Array":false},"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,"common_handle_version":0,"comment":"","auto_inc_id":0,"auto_id_cache":0,"auto_rand_id":0,"max_col_id":8,"max_idx_id":0,"max_fk_id":0,"max_cst_id":0,"update_timestamp":452653255976550448,"ShardRowIDBits":0,"max_shard_row_id_bits":0,"auto_random_bits":0,"auto_random_range_bits":0,"pre_split_regions":0,"partition":null,"compression":"","view":null,"sequence":null,"Lock":null,"version":5,"tiflash_replica":{"Count":1,"LocationLabels":[],"Available":false,"AvailablePartitionIDs":null},"is_columnar":false,"temp_table_type":0,"cache_table_status":0,"policy_ref_info":null,"stats_options":null,"exchange_partition_info":null,"ttl_info":null,"revision":1})json", //
+            R"stmt(CREATE TABLE `db_2`.`t_546`(`col_86` Nullable(String), `col_87` MyDateTime(0), `col_88` Nullable(UInt64), `col_89` String, `col_90` UInt8, `col_91` Nullable(UInt64), `col_92` String, `col_93` Nullable(String), `_tidb_rowid` Int64) Engine = DeltaMerge((`_tidb_rowid`), '{"cols":[{"comment":"","default":null,"default_bit":null,"id":1,"name":{"L":"col_86","O":"col_86"},"offset":0,"origin_default":null,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Elems":null,"Flag":128,"Flen":65535,"Tp":252}},{"comment":"","default":"1994-05-0600:00:00","default_bit":null,"id":2,"name":{"L":"col_87","O":"col_87"},"offset":1,"origin_default":null,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Elems":null,"Flag":129,"Flen":19,"Tp":12}},{"comment":"","default":null,"default_bit":null,"id":3,"name":{"L":"col_88","O":"col_88"},"offset":2,"origin_default":null,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Elems":null,"Flag":32,"Flen":42,"Tp":16}},{"comment":"","default":"\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000","default_bit":null,"id":4,"name":{"L":"col_89","O":"col_89"},"offset":3,"origin_default":null,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Elems":null,"Flag":129,"Flen":21,"Tp":254}},{"comment":"","default":null,"default_bit":null,"id":5,"name":{"L":"col_90","O":"col_90"},"offset":4,"origin_default":null,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Elems":null,"Flag":4129,"Flen":3,"Tp":1}},{"comment":"","default":"\\u0007\\u0007","default_bit":"Bwc=","id":6,"name":{"L":"col_91","O":"col_91"},"offset":5,"origin_default":null,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Elems":null,"Flag":32,"Flen":12,"Tp":16}},{"comment":"","default":"kY~6to6H4ut*QAPrj@&","default_bit":null,"id":7,"name":{"L":"col_92","O":"col_92"},"offset":6,"origin_default":null,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Elems":null,"Flag":129,"Flen":343,"Tp":15}},{"comment":"","default":null,"default_bit":null,"id":8,"name":{"L":"col_93","O":"col_93"},"offset":7,"origin_default":null,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Elems":null,"Flag":128,"Flen":-1,"Tp":245}}],"comment":"","id":546,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"tcfc7825f","O":"tcfc7825f"},"partition":null,"pk_is_handle":false,"schema_version":-1,"state":5,"tiflash_replica":{"Available":false,"Count":1},"update_timestamp":452653255976550448}', 0))stmt", //
+        },
     };
+    // clang-format on
 
     for (const auto & c : cases)
     {
+4 −2		Foundation/include/Poco/JSONString.h
+6 −2		Foundation/include/Poco/UTF8String.h
+36 −4		Foundation/src/JSONString.cpp
+5 −5		Foundation/src/UTF8String.cpp
+3 −5		Foundation/src/VarHolder.cpp
+12 −2		Foundation/testsuite/src/StringTest.cpp
+21 −0		Foundation/testsuite/src/UTF8StringTest.cpp
+2 −0		Foundation/testsuite/src/UTF8StringTest.h
+29 −3		JSON/include/Poco/JSON/Array.h
+119 −55		JSON/include/Poco/JSON/Object.h
+10 −7		JSON/include/Poco/JSON/Stringifier.h
+4 −4		JSON/src/Array.cpp
+57 −27		JSON/src/Object.cpp
+13 −9		JSON/src/Stringifier.cpp
+184 −6		JSON/testsuite/src/JSONTest.cpp
+3 −1		JSON/testsuite/src/JSONTest.h