ddl: fix TypeBit default value (#9467)

dbms/src/Storages/Transaction/TiDB.cpp

@@ -148,14 +148,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())
@@ -366,6 +359,8 @@ try
     json->set("offset", offset);
     json->set("origin_default", origin_default_value);
     json->set("default", default_value);
+    if (!origin_default_bit_value.isEmpty())
+        json->set("origin_default_bit", origin_default_bit_value);
     json->set("default_bit", default_bit_value);
     Poco::JSON::Object::Ptr tp_json = new Poco::JSON::Object();
     tp_json->set("Tp", static_cast<Int32>(tp));
@@ -414,6 +409,8 @@ try
         origin_default_value = json->get("origin_default");
     if (!json->isNull("default"))
         default_value = json->get("default");
+    if (!json->isNull("origin_default_bit"))
+        origin_default_bit_value = json->get("origin_default_bit");
     if (!json->isNull("default_bit"))
         default_bit_value = json->get("default_bit");
     auto type_json = json->getObject("type");
@@ -1215,6 +1212,14 @@ ColumnInfo toTiDBColumnInfo(const tipb::ColumnInfo & tipb_column_info)
     tidb_column_info.flag = tipb_column_info.flag();
     tidb_column_info.flen = tipb_column_info.columnlen();
     tidb_column_info.decimal = tipb_column_info.decimal();
+    // 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
+    // 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();
     for (int i = 0; i < tipb_column_info.elems_size(); ++i)
         tidb_column_info.elems.emplace_back(tipb_column_info.elems(i), i + 1);
     return tidb_column_info;

dbms/src/Storages/Transaction/TiDB.h

@@ -185,6 +185,7 @@ struct ColumnInfo
     String name;
     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;

dbms/src/Storages/Transaction/tests/gtest_table_info.cpp

@@ -30,12 +30,10 @@ using DBInfo = TiDB::DBInfo;
 
 namespace DB
 {
-
 String createTableStmt(const DBInfo & db_info, const TableInfo & table_info, const SchemaNameMapper & name_mapper, const LoggerPtr & log);
 
 namespace tests
 {
-
 struct ParseCase
 {
     String table_info_json;
@@ -143,13 +141,13 @@ try
              1145, //
              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 `customer`.`customerdebt`(`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}'))stmt", //
+             R"stmt(CREATE TABLE `customer`.`customerdebt`(`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}'))stmt", //
          },
          StmtCase{
              2049, //
              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 `customer`.`customerdebt`(`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}'))stmt", //
+             R"stmt(CREATE TABLE `customer`.`customerdebt`(`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}'))stmt", //
          },
          StmtCase{
              31, //

tests/fullstack-test2/ddl/alter_column_bit.test

@@ -0,0 +1,62 @@
+# Copyright 2024 PingCAP, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+mysql> drop table if exists test.t;
+mysql> create table test.t(id int(11) NOT NULL, a bit(4) DEFAULT b'0', b bit(12) DEFAULT b'11100000111', c bit(12) DEFAULT b'111100001000', PRIMARY KEY (id));
+mysql> insert into test.t (id) values (1),(2),(3);
+mysql> alter table test.t set tiflash replica 1;
+
+func> wait_table test t
+
+mysql_bin_as_hex> set session tidb_isolation_read_engines='tiflash'; select * from test.t order by id;
++----+------------+------------+------------+
+| id | a          | b          | c          |
++----+------------+------------+------------+
+|  1 | 0x00       | 0x0707     | 0x0F08     |
+|  2 | 0x00       | 0x0707     | 0x0F08     |
+|  3 | 0x00       | 0x0707     | 0x0F08     |
++----+------------+------------+------------+
+
+mysql> alter table test.t modify column a bit(4) default b'1';
+mysql_bin_as_hex> set session tidb_isolation_read_engines='tiflash'; select * from test.t order by id;
++----+------------+------------+------------+
+| id | a          | b          | c          |
++----+------------+------------+------------+
+|  1 | 0x00       | 0x0707     | 0x0F08     |
+|  2 | 0x00       | 0x0707     | 0x0F08     |
+|  3 | 0x00       | 0x0707     | 0x0F08     |
++----+------------+------------+------------+
+
+mysql> alter table test.t modify column a bit(4) default b'0111';
+mysql_bin_as_hex> set session tidb_isolation_read_engines='tiflash'; select * from test.t order by id;
++----+------------+------------+------------+
+| id | a          | b          | c          |
++----+------------+------------+------------+
+|  1 | 0x00       | 0x0707     | 0x0F08     |
+|  2 | 0x00       | 0x0707     | 0x0F08     |
+|  3 | 0x00       | 0x0707     | 0x0F08     |
++----+------------+------------+------------+
+
+mysql> insert into test.t (id) values (4);
+mysql_bin_as_hex> set session tidb_isolation_read_engines='tiflash'; select * from test.t order by id;
++----+------------+------------+------------+
+| id | a          | b          | c          |
++----+------------+------------+------------+
+|  1 | 0x00       | 0x0707     | 0x0F08     |
+|  2 | 0x00       | 0x0707     | 0x0F08     |
+|  3 | 0x00       | 0x0707     | 0x0F08     |
+|  4 | 0x07       | 0x0707     | 0x0F08     |
++----+------------+------------+------------+
+
+mysql> drop table if exists test.t;

tests/run-test.py

@@ -21,6 +21,7 @@
 import re
 import sys
 import time
+import datetime
 
 if sys.version_info.major == 2:
     # print('running with py2: {}.{}.{}'.format(sys.version_info.major, sys.version_info.minor, sys.version_info.micro))
@@ -35,6 +36,7 @@
 CMD_PREFIX = '>> '
 CMD_PREFIX_ALTER = '=> '
 CMD_PREFIX_TIDB = 'mysql> '
+CMD_PREFIX_TIDB_BINALY_AS_HEX = 'mysql_bin_as_hex> '
 CMD_PREFIX_FUNC = 'func> '
 RETURN_PREFIX = '#RETURN'
 SLEEP_PREFIX = 'SLEEP '
@@ -69,9 +71,10 @@ class Executor:
     def __init__(self, dbc):
         self.dbc = dbc
 
-    def exe(self, cmd, unescape = True):
-        if unescape:
-            cmd = self.dbc + ' "' + to_unescaped_str(cmd) + '" 2>&1'
+    def exe(self, cmd, unescape = True, binary_as_hex = False):
+        cmd = to_unescaped_str(cmd) if unescape else cmd
+        if binary_as_hex:
+            cmd = self.dbc + ' "' + cmd + '" --binary-as-hex=true 2>&1'
         else:
             cmd = self.dbc + ' "' + cmd + '" 2>&1'
         return exec_func(cmd)
@@ -277,24 +280,27 @@ def __init__(self, executor, executor_tidb, executor_func, executor_curl_tidb, f
     def on_line(self, line, line_number):
         if line.startswith(SLEEP_PREFIX):
             time.sleep(float(line[len(SLEEP_PREFIX):]))
-        elif line.startswith(CMD_PREFIX_TIDB):
+        elif line.startswith(CMD_PREFIX_TIDB) or line.startswith(CMD_PREFIX_TIDB_BINALY_AS_HEX):
             unescape_flag = True
             if line.endswith(NO_UNESCAPE_SUFFIX):
                 unescape_flag = False
                 line = line[:-len(NO_UNESCAPE_SUFFIX)]
-            if verbose: print('running', line)
+            if verbose: print('{} running {}'.format(datetime.datetime.now().strftime('%H:%M:%S.%f'), line))
             if self.outputs != None and ((not self.is_mysql and not matched(self.outputs, self.matches, self.fuzz)) or (
                 self.is_mysql and not MySQLCompare.matched(self.outputs, self.matches))):
                 return False
             self.query_line_number = line_number
             self.is_mysql = True
-            self.query = line[len(CMD_PREFIX_TIDB):]
+            if line.startswith(CMD_PREFIX_TIDB_BINALY_AS_HEX):
+                self.query = line[len(CMD_PREFIX_TIDB_BINALY_AS_HEX):]
+            else:
+                self.query = line[len(CMD_PREFIX_TIDB):]
             # for mysql commands ignore errors since they may be part of the test logic.
-            self.outputs, _ = self.executor_tidb.exe(self.query, unescape_flag)
+            self.outputs, _ = self.executor_tidb.exe(self.query, unescape_flag, line.startswith(CMD_PREFIX_TIDB_BINALY_AS_HEX))
             self.outputs = [x.strip() for x in self.outputs if len(x.strip()) != 0]
             self.matches = []
         elif line.startswith(CURL_TIDB_STATUS_PREFIX):
-            if verbose: print('running', line)
+            if verbose: print('{} running {}'.format(datetime.datetime.now().strftime('%H:%M:%S.%f'), line))
             if self.outputs != None and ((not self.is_mysql and not matched(self.outputs, self.matches, self.fuzz)) or (
                 self.is_mysql and not MySQLCompare.matched(self.outputs, self.matches))):
                 return False
@@ -306,7 +312,7 @@ def on_line(self, line, line_number):
                 return False
             self.matches = []
         elif line.startswith(CMD_PREFIX) or line.startswith(CMD_PREFIX_ALTER):
-            if verbose: print('running', line)
+            if verbose: print('{} running {}'.format(datetime.datetime.now().strftime('%H:%M:%S.%f'), line))
             if self.outputs != None and ((not self.is_mysql and not matched(self.outputs, self.matches, self.fuzz)) or (
                 self.is_mysql and not MySQLCompare.matched(self.outputs, self.matches))):
                 return False
@@ -320,7 +326,7 @@ def on_line(self, line, line_number):
                 self.outputs = [x for x in self.outputs if x.find(ignored_output) < 0]
             self.matches = []
         elif line.startswith(CMD_PREFIX_FUNC):
-            if verbose: print('running', line)
+            if verbose: print('{} running {}'.format(datetime.datetime.now().strftime('%H:%M:%S.%f'), line))
             if self.outputs != None and ((not self.is_mysql and not matched(self.outputs, self.matches, self.fuzz)) or (
                 self.is_mysql and not MySQLCompare.matched(self.outputs, self.matches))):
                 return False