Skip to content

Commit bbda147

Browse files
ti-chi-botJaySon-Huang
ti-chi-bot
and
JaySon-Huang
authored
ddl: Fix compatibility with null expression index (#10162) (#10174)
close #9891 ddl: Fix TiFlash panic when meets expression index with format `((NULL))` Using `ColumnNothing` in IStorage may bring unexpected behavior when we try to write or read the column. So TiFlash create `ColumnInt8` instead of `ColumnNothing` when `TiDB::ColumnInfo.Tp == Null`. However, we don't expected the column is used when reading or writing to TiFlash. * `((null))` is not a normal expression index, this could be created by the user in accident (e.g. executing wrong SQL statement to create the expression index), TiDB should not write or read from it * Even for normal expression index, TiDB does not use expression when accessing to TiFlash Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> Signed-off-by: JaySon-Huang <tshent@qq.com> Co-authored-by: JaySon <tshent@qq.com> Co-authored-by: JaySon-Huang <tshent@qq.com>
1 parent 29c9d05 commit bbda147

5 files changed

Lines changed: 172 additions & 6 deletions

File tree

‎dbms/src/Storages/DeltaMerge/tests/gtest_dm_delta_merge_store.cpp‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <Common/MyTime.h>
1818
#include <Common/SyncPoint/SyncPoint.h>
1919
#include <DataTypes/DataTypeMyDateTime.h>
20+
#include <DataTypes/DataTypesNumber.h>
2021
#include <Interpreters/Context.h>
2122
#include <Storages/DeltaMerge/DeltaMergeDefines.h>
2223
#include <Storages/DeltaMerge/DeltaMergeStore.h>
@@ -2219,6 +2220,84 @@ try
22192220
}
22202221
CATCH
22212222

2223+
TEST_P(DeltaMergeStoreRWTest, DDLAddColumnInvalidExpressionIndex)
2224+
try
2225+
{
2226+
// const String col_name_to_add = "_v$_idx_name_0";
2227+
// const DataTypePtr col_type_to_add = DataTypeFactory::instance().getOrSet("Int8");
2228+
2229+
// write some rows before DDL
2230+
size_t num_rows_write = 1;
2231+
{
2232+
Block block = DMTestEnv::prepareSimpleWriteBlock(0, num_rows_write, false);
2233+
store->write(*db_context, db_context->getSettingsRef(), block);
2234+
}
2235+
2236+
// DDL add column for invalid expression index
2237+
// actual ddl is like: ADD COLUMN `_v$_idx_name_0` Nullable(Int8)
2238+
{
2239+
TiDB::TableInfo new_table_info;
2240+
static const String json_table_info = R"(
2241+
{"cols":[{"comment": "","default": null,"default_bit": null,"id":1,"name":{"L":"id","O":"id"},"offset":0,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":11,"Tp":3}},{"comment": "","default": null,"default_bit": null,"id":2,"name":{"L":"_v$_idx_name_0","O":"_v$_idx_name_0"},"offset":1,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":136,"Flen":0,"Tp":6}}],"comment": "","id":242807,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"t","O":"t"},"pk_is_handle":false,"schema_version":-1,"state":5,"tiflash_replica":{"Available":false,"Count":1},"update_timestamp":456163970651521027}
2242+
)";
2243+
new_table_info.deserialize(json_table_info);
2244+
store->applySchemaChanges(new_table_info);
2245+
}
2246+
2247+
// try read
2248+
{
2249+
ColumnDefines cols_to_read;
2250+
bool has_v_index = false;
2251+
bool has_id = false;
2252+
for (const auto & col : store->getTableColumns())
2253+
{
2254+
if (col.name == DMTestEnv::pk_name)
2255+
{
2256+
cols_to_read.emplace_back(col);
2257+
}
2258+
else if (col.name == "id")
2259+
{
2260+
cols_to_read.emplace_back(col);
2261+
has_id = true;
2262+
}
2263+
else if (col.name == "_v$_idx_name_0")
2264+
{
2265+
// we check the type of this column, but not read from it
2266+
ASSERT_EQ(col.type->getName(), "Nullable(Int8)") << store->getHeader()->dumpJsonStructure();
2267+
has_v_index = true;
2268+
}
2269+
}
2270+
ASSERT_TRUE(has_id) << store->getHeader()->dumpJsonStructure();
2271+
ASSERT_TRUE(has_v_index) << store->getHeader()->dumpJsonStructure();
2272+
2273+
auto in = store->read(
2274+
*db_context,
2275+
db_context->getSettingsRef(),
2276+
cols_to_read,
2277+
{RowKeyRange::newAll(store->isCommonHandle(), store->getRowKeyColumnSize())},
2278+
/* num_streams= */ 1,
2279+
/* start_ts= */ std::numeric_limits<UInt64>::max(),
2280+
EMPTY_FILTER,
2281+
std::vector<RuntimeFilterPtr>{},
2282+
0,
2283+
TRACING_NAME,
2284+
/* keep_order= */ false,
2285+
/* is_fast_scan= */ false,
2286+
/* expected_block_size= */ 1024)[0];
2287+
ASSERT_UNORDERED_INPUTSTREAM_COLS_UR(
2288+
in,
2289+
Strings({DMTestEnv::pk_name, "id"}),
2290+
createColumns({
2291+
createColumn<Int64>(createNumbers<Int64>(0, num_rows_write)),
2292+
// all "id" are NULL
2293+
createNullableColumn<Int32>(
2294+
std::vector<Int64>(num_rows_write, 0),
2295+
std::vector<Int32>(num_rows_write, 1)),
2296+
}));
2297+
}
2298+
}
2299+
CATCH
2300+
22222301
TEST_P(DeltaMergeStoreRWTest, DDLRenameColumn)
22232302
try
22242303
{

‎dbms/src/TiDB/Decode/TypeMapping.cpp‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,22 @@ DataTypePtr TypeMapping::getDataType(const ColumnInfo & column_info)
179179
// This does not support the "duration" type.
180180
DataTypePtr getDataTypeByColumnInfo(const ColumnInfo & column_info)
181181
{
182-
DataTypePtr base = TypeMapping::instance().getDataType(column_info);
182+
DataTypePtr base;
183+
if (likely(column_info.tp != TiDB::TP::TypeNull))
184+
{
185+
base = TypeMapping::instance().getDataType(column_info);
186+
}
187+
else
188+
{
189+
// Storing a column with `ColumnNothing` is not allowed in `StorageFactory::get/checkAllTypesAreAllowedInTable`
190+
// Using `ColumnNothing` in IStorage may bring unexpected behavior when we
191+
// try to write or read the column. So we change it to `ColumnInt8`.
192+
LOG_WARNING(
193+
Logger::get(),
194+
"Column type is TiDB::TP::TypeNull, change it to Int8 for compatibility, column_id={}",
195+
column_info.id);
196+
base = DataTypeFactory::instance().getOrSet("Int8");
197+
}
183198

184199
if (!column_info.hasNotNullFlag())
185200
{

dbms/src/TiDB/tests/gtest_table_info.cpp renamed to dbms/src/TiDB/Schema/tests/gtest_table_info.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,16 @@ try
121121
getDataTypeByColumnInfo(ci);
122122
}
123123
}},
124+
// tiflash#9891, expression index like `KEY idx_name ((null))` generate column in this format
125+
ParseCase{
126+
R"json({"cols":[{"comment": "","default": null,"default_bit": null,"id":1,"name":{"L":"id","O":"id"},"offset":0,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":11,"Tp":3}},{"comment": "","default": null,"default_bit": null,"id":2,"name":{"L":"_v$_idx_name_0","O":"_v$_idx_name_0"},"offset":1,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":136,"Flen":0,"Tp":6}}],"comment": "","id":242807,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"t","O":"t"},"pk_is_handle":false,"schema_version":-1,"state":5,"tiflash_replica":{"Available":false,"Count":1},"update_timestamp":456163970651521027})json",
127+
[](const TableInfo & table_info) {
128+
ASSERT_EQ(table_info.name, "t");
129+
auto col_1 = table_info.getColumnInfo(2);
130+
ASSERT_EQ(col_1.tp, TiDB::TP::TypeNull);
131+
ASSERT_TRUE(col_1.hasMultipleKeyFlag());
132+
ASSERT_TRUE(col_1.hasBinaryFlag());
133+
}},
124134
};
125135

126136
for (const auto & c : cases)
@@ -247,6 +257,14 @@ try
247257
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", //
248258
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", //
249259
},
260+
StmtCase{
261+
1145, //
262+
0,
263+
R"json({"id":1939,"db_name":{"O":"customer","L":"customer"},"charset":"utf8mb4","collate":"utf8mb4_bin","state":5})json", //
264+
R"json({"cols":[{"comment": "","default": null,"default_bit": null,"id":1,"name":{"L":"id","O":"id"},"offset":0,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":0,"Flen":11,"Tp":3}},{"comment": "","default": null,"default_bit": null,"id":2,"name":{"L":"_v$_idx_name_0","O":"_v$_idx_name_0"},"offset":1,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Flag":136,"Flen":0,"Tp":6}}],"comment": "","id":242807,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"t","O":"t"},"pk_is_handle":false,"schema_version":-1,"state":5,"tiflash_replica":{"Available":false,"Count":1},"update_timestamp":456163970651521027})json", //
265+
// Note that the `v$_idx_name_0` column is created as Nullable(Int8) rather than Nullable(Nothing) for compatibility with null expression index (tiflash#9891)
266+
R"stmt(CREATE TABLE `db_1939`.`t_242807`(`id` Nullable(Int32), `_v$_idx_name_0` Nullable(Int8), `_tidb_rowid` Int64) Engine = DeltaMerge((`_tidb_rowid`), '{"cols":[{"comment":"","default":null,"default_bit":null,"id":1,"name":{"L":"id","O":"id"},"offset":0,"origin_default":null,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Elems":null,"Flag":0,"Flen":11,"Tp":3}},{"comment":"","default":null,"default_bit":null,"id":2,"name":{"L":"_v$_idx_name_0","O":"_v$_idx_name_0"},"offset":1,"origin_default":null,"state":5,"type":{"Charset":"binary","Collate":"binary","Decimal":0,"Elems":null,"Flag":136,"Flen":0,"Tp":6}}],"comment":"","id":242807,"index_info":[],"is_common_handle":false,"keyspace_id":4294967295,"name":{"L":"t","O":"t"},"partition":null,"pk_is_handle":false,"schema_version":-1,"state":5,"tiflash_replica":{"Available":false,"Count":1},"update_timestamp":456163970651521027}', 0))stmt", //
267+
},
250268
};
251269
// clang-format on
252270

‎dbms/src/TiDB/tests/gtest_type_mapping.cpp‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@
1717
#include <TiDB/Decode/TypeMapping.h>
1818
#include <TiDB/Schema/TiDB.h>
1919

20-
namespace DB
21-
{
22-
namespace tests
20+
namespace DB::tests
2321
{
2422

2523
TEST(TypeMappingTest, DataTypeToColumnInfo)
@@ -78,9 +76,18 @@ try
7876
auto data_type = getDataTypeByColumnInfo(column_info);
7977
ASSERT_EQ(data_type->getName(), "String");
8078

79+
{
80+
auto str_type = typeFromString("Int8");
81+
column_info = reverseGetColumnInfo(NameAndTypePair{name, str_type}, 1, default_field, true);
82+
column_info.tp = TiDB::TP::TypeNull; // test for TypeNull
83+
ASSERT_EQ(column_info.tp, TiDB::TP::TypeNull);
84+
auto data_type = getDataTypeByColumnInfo(column_info);
85+
// Use Int8 to compatible "storing" TypeNull in IStorage
86+
ASSERT_EQ(data_type->getName(), "Int8");
87+
}
88+
8189
// TODO: test decimal, datetime, enum
8290
}
8391
CATCH
8492

85-
} // namespace tests
86-
} // namespace DB
93+
} // namespace DB::tests

‎tests/fullstack-test2/ddl/expression_index.test‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
# Copyright 2025 PingCAP, Inc.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
mysql> drop table if exists test.t;
16+
17+
# test for null expression express tiflash#9891
18+
mysql> CREATE TABLE test.t (id int, KEY idx_name ((null)));
19+
mysql> alter table test.t set tiflash replica 1;
20+
mysql> insert test.t values(0),(1);
21+
22+
func> wait_table test t
23+
24+
mysql> insert into test.t values (2), (3);
25+
mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.t order by id;
26+
+----+
27+
| id |
28+
+----+
29+
| 0 |
30+
| 1 |
31+
| 2 |
32+
| 3 |
33+
+----+
34+
35+
mysql> CREATE INDEX idx_n ON test.t ((null));
36+
mysql> alter table test.t add column c1 int;
37+
mysql> set session tidb_isolation_read_engines='tiflash'; select id,c1 from test.t order by id;
38+
+------+------+
39+
| id | c1 |
40+
+------+------+
41+
| 0 | NULL |
42+
| 1 | NULL |
43+
| 2 | NULL |
44+
| 3 | NULL |
45+
+------+------+
46+
47+
mysql> drop table if exists test.t;

0 commit comments

Comments
 (0)