Skip to content

Commit 1c43a37

Browse files
fix virtual column not found when remote read happens (#9920) (#9924)
close #9561 Signed-off-by: guo-shaoge <shaoge1994@163.com> Co-authored-by: guo-shaoge <shaoge1994@163.com>
1 parent ca5927a commit 1c43a37

4 files changed

Lines changed: 61 additions & 3 deletions

File tree

dbms/src/Common/FailPoint.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ namespace DB
8787
M(force_set_page_data_compact_batch) \
8888
M(force_set_dtfile_exist_when_acquire_id) \
8989
M(force_no_local_region_for_mpp_task) \
90+
M(force_random_remote_read) \
9091
M(force_remote_read_for_batch_cop) \
9192
M(force_pd_grpc_error) \
9293
M(force_context_path) \

dbms/src/Flash/Coprocessor/RemoteRequest.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// limitations under the License.
1414

1515
#include <Common/FmtUtils.h>
16+
#include <DataStreams/GeneratedColumnPlaceholderBlockInputStream.h>
1617
#include <Flash/Coprocessor/ChunkCodec.h>
1718
#include <Flash/Coprocessor/RemoteRequest.h>
1819
#include <Storages/MutableSupport.h>
@@ -60,8 +61,14 @@ RemoteRequest RemoteRequest::build(
6061
{
6162
const auto & col = table_scan.getColumns()[i];
6263
auto col_id = col.column_id();
64+
auto tidb_ci = TiDB::toTiDBColumnInfo(col);
6365

64-
if (col_id == DB::TiDBPkColumnID)
66+
if (tidb_ci.hasGeneratedColumnFlag())
67+
{
68+
const auto & col_name = GeneratedColumnPlaceholderBlockInputStream::getColumnName(i);
69+
schema.emplace_back(std::make_pair(col_name, std::move(tidb_ci)));
70+
}
71+
else if (col_id == DB::TiDBPkColumnID)
6572
{
6673
ColumnInfo ci;
6774
ci.tp = TiDB::TypeLongLong;

dbms/src/Flash/Coprocessor/TablesRegionsInfo.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ namespace DB
2323
namespace FailPoints
2424
{
2525
extern const char force_no_local_region_for_mpp_task[];
26+
extern const char force_random_remote_read[];
2627
} // namespace FailPoints
2728

2829
SingleTableRegions & TablesRegionsInfo::getOrCreateTableRegionInfoByTableID(Int64 table_id)
@@ -57,8 +58,9 @@ static bool needRemoteRead(const RegionInfo & region_info, const TMTContext & tm
5758
static void insertRegionInfoToTablesRegionInfo(const google::protobuf::RepeatedPtrField<coprocessor::RegionInfo> & regions, Int64 table_id, TablesRegionsInfo & tables_region_infos, std::unordered_set<RegionID> & local_region_id_set, const TMTContext & tmt_context)
5859
{
5960
auto & table_region_info = tables_region_infos.getOrCreateTableRegionInfoByTableID(table_id);
60-
for (const auto & r : regions)
61+
for (int i = 0; i < regions.size(); ++i) // NOLINT
6162
{
63+
const auto & r = regions[i];
6264
RegionInfo region_info(r.region_id(), r.region_epoch().version(), r.region_epoch().conf_ver(), CoprocessorHandler::genCopKeyRange(r.ranges()), nullptr);
6365
if (region_info.key_ranges.empty())
6466
{
@@ -76,8 +78,15 @@ static void insertRegionInfoToTablesRegionInfo(const google::protobuf::RepeatedP
7678
/// 4. The remote read will fetch the newest region info via key ranges. So it is possible to find the region
7779
/// is served by the same node (but still read from remote).
7880
bool duplicated_region = local_region_id_set.count(region_info.region_id) > 0;
81+
bool is_remote = duplicated_region || needRemoteRead(region_info, tmt_context);
82+
#ifndef NDEBUG
83+
fiu_do_on(FailPoints::force_random_remote_read, {
84+
if ((i % 2) != 0)
85+
is_remote = true;
86+
});
87+
#endif
7988

80-
if (duplicated_region || needRemoteRead(region_info, tmt_context))
89+
if (is_remote)
8190
table_region_info.remote_regions.push_back(region_info);
8291
else
8392
{
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
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+
# Preparation.
16+
=> DBGInvoke __init_fail_point()
17+
18+
mysql> drop table if exists test.test_vir;
19+
mysql> create table test.test_vir (id int not null, str varchar(100), str_vir varchar(100) as (lower(str)) VIRTUAL);
20+
mysql> insert into test.test_vir (id, str) values(1, 'ABC'), (2, 'Def'), (3, 'xXXX');
21+
mysql> alter table test.test_vir set tiflash replica 1;
22+
mysql> set @@cte_max_recursion_depth = 10000000; insert into test.test_vir (id, str) with recursive cte1 as (select 1 c1, 'a' c2 union select c1+1 c1, substr(concat(c2, 'B'), 10) c2 from cte1 limit 10000) select * from cte1;
23+
24+
func> wait_table test test_vir
25+
mysql> split table test.test_vir by (100), (500), (1000);
26+
TOTAL_SPLIT_REGION SCATTER_FINISH_RATIO
27+
3 1
28+
29+
=> DBGInvoke __enable_fail_point(force_random_remote_read)
30+
mysql> set @@tidb_isolation_read_engines='tiflash'; set @@tidb_enforce_mpp=1; select * from test.test_vir order by 1, 2, 3 limit 5;
31+
id str str_vir
32+
1 ABC abc
33+
1 a a
34+
2
35+
2 Def def
36+
3
37+
=> DBGInvoke __disable_fail_point(force_random_remote_read)
38+
39+
# Clean up.
40+
mysql> drop table if exists test.test_vir
41+

0 commit comments

Comments
 (0)