diff --git a/contrib/poco b/contrib/poco index 170872b2a15..65c2f19739d 160000 --- a/contrib/poco +++ b/contrib/poco @@ -1 +1 @@ -Subproject commit 170872b2a15ea042d99f8168a01aec896eef0840 +Subproject commit 65c2f19739da722d56f866bbaf3472bc6a6e8af7 diff --git a/dbms/src/Storages/Transaction/TiDB.cpp b/dbms/src/Storages/Transaction/TiDB.cpp index 3bdd32761e6..4cb80489597 100644 --- a/dbms/src/Storages/Transaction/TiDB.cpp +++ b/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(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; diff --git a/dbms/src/Storages/Transaction/TiDB.h b/dbms/src/Storages/Transaction/TiDB.h index 6c6b4f252b1..2efa185cecb 100644 --- a/dbms/src/Storages/Transaction/TiDB.h +++ b/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; diff --git a/dbms/src/Storages/Transaction/tests/gtest_table_info.cpp b/dbms/src/Storages/Transaction/tests/gtest_table_info.cpp index fb02266b8f3..668f54eac5b 100644 --- a/dbms/src/Storages/Transaction/tests/gtest_table_info.cpp +++ b/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, // diff --git a/tests/fullstack-test2/ddl/alter_column_bit.test b/tests/fullstack-test2/ddl/alter_column_bit.test new file mode 100644 index 00000000000..18aa5f206fb --- /dev/null +++ b/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; diff --git a/tests/run-test.py b/tests/run-test.py index 06cf55baf98..b6719727960 100644 --- a/tests/run-test.py +++ b/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