Skip to content

Commit c529adf

Browse files
ti-chi-botgengliqi
andauthored
Fix three schema mismatch bugs under disaggregated arch (#10530) (#10579)
close #10226 Fix three schema mismatch bugs under disaggregated arch Signed-off-by: gengliqi <gengliqiii@gmail.com> Co-authored-by: Liqi Geng <gengliqiii@gmail.com>
1 parent a8a304a commit c529adf

10 files changed

Lines changed: 126 additions & 57 deletions

File tree

dbms/src/Flash/Coprocessor/GenSchemaAndColumn.cpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15+
#include <DataStreams/GeneratedColumnPlaceholderBlockInputStream.h>
1516
#include <Flash/Coprocessor/GenSchemaAndColumn.h>
1617
#include <Storages/DeltaMerge/DeltaMergeDefines.h>
1718
#include <Storages/MutableSupport.h>
@@ -94,14 +95,23 @@ NamesAndTypes genNamesAndTypes(const TiDBTableScan & table_scan, const StringRef
9495
return genNamesAndTypes(table_scan.getColumns(), column_prefix);
9596
}
9697

97-
std::tuple<DM::ColumnDefinesPtr, int> genColumnDefinesForDisaggregatedRead(const TiDBTableScan & table_scan)
98+
std::tuple<DM::ColumnDefinesPtr, int, std::vector<std::tuple<UInt64, String, DataTypePtr>>> genColumnDefinesForDisaggregatedRead(
99+
const TiDBTableScan & table_scan)
98100
{
99101
auto column_defines = std::make_shared<DM::ColumnDefines>();
100102
int extra_table_id_index = MutSup::invalid_col_id;
101103
column_defines->reserve(table_scan.getColumnSize());
104+
std::vector<std::tuple<UInt64, String, DataTypePtr>> generated_column_infos;
102105
for (Int32 i = 0; i < table_scan.getColumnSize(); ++i)
103106
{
104107
const auto & column_info = table_scan.getColumns()[i];
108+
if (column_info.hasGeneratedColumnFlag())
109+
{
110+
const auto & data_type = getDataTypeByColumnInfoForComputingLayer(column_info);
111+
const auto & col_name = GeneratedColumnPlaceholderBlockInputStream::getColumnName(i);
112+
generated_column_infos.push_back(std::make_tuple(i, col_name, data_type));
113+
continue;
114+
}
105115
// Now the upper level seems treat disagg read as an ExchangeReceiver output, so
106116
// use this as output column prefix.
107117
// Even if the id is pk_column or extra_table_id, we still output it as
@@ -117,10 +127,6 @@ std::tuple<DM::ColumnDefinesPtr, int> genColumnDefinesForDisaggregatedRead(const
117127
break;
118128
case MutSup::extra_table_id_col_id:
119129
{
120-
column_defines->emplace_back(DM::ColumnDefine{
121-
MutSup::extra_table_id_col_id,
122-
output_name, // MutSup::extra_table_id_column_name
123-
MutSup::getExtraTableIdColumnType()});
124130
extra_table_id_index = i;
125131
break;
126132
}
@@ -133,7 +139,7 @@ std::tuple<DM::ColumnDefinesPtr, int> genColumnDefinesForDisaggregatedRead(const
133139
break;
134140
}
135141
}
136-
return {std::move(column_defines), extra_table_id_index};
142+
return {std::move(column_defines), extra_table_id_index, std::move(generated_column_infos)};
137143
}
138144

139145
ColumnsWithTypeAndName getColumnWithTypeAndName(const NamesAndTypes & names_and_types)

dbms/src/Flash/Coprocessor/GenSchemaAndColumn.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ NamesAndTypes genNamesAndTypes(const TiDB::ColumnInfos & column_infos, const Str
3434
ColumnsWithTypeAndName getColumnWithTypeAndName(const NamesAndTypes & names_and_types);
3535
NamesAndTypes toNamesAndTypes(const DAGSchema & dag_schema);
3636

37-
// The column defines and `extra table id index`
38-
std::tuple<DM::ColumnDefinesPtr, int> genColumnDefinesForDisaggregatedRead(const TiDBTableScan & table_scan);
37+
// The column defines, `extra table id index` and `generated columns info` for disaggregated read.
38+
std::tuple<DM::ColumnDefinesPtr, int, std::vector<std::tuple<UInt64, String, DataTypePtr>>> genColumnDefinesForDisaggregatedRead(
39+
const TiDBTableScan & table_scan);
3940

4041
} // namespace DB

dbms/src/Storages/StorageDisaggregated.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ class StorageDisaggregated : public IStorage
108108
std::tuple<DM::RSOperatorPtr, DM::ColumnRangePtr> buildRSOperatorAndColumnRange(
109109
const Context & db_context,
110110
const DM::ColumnDefinesPtr & columns_to_read);
111-
std::variant<DM::Remote::RNWorkersPtr, DM::SegmentReadTaskPoolPtr> packSegmentReadTasks(
111+
std::tuple<std::variant<DM::Remote::RNWorkersPtr, DM::SegmentReadTaskPoolPtr>, DM::ColumnDefinesPtr> packSegmentReadTasks(
112112
const Context & db_context,
113113
DM::SegmentReadTasks && read_tasks,
114114
const DM::ColumnDefinesPtr & column_defines,
@@ -161,5 +161,7 @@ class StorageDisaggregated : public IStorage
161161
std::unique_ptr<DAGExpressionAnalyzer> analyzer;
162162
static constexpr auto ZONE_LABEL_KEY = "zone";
163163
std::optional<String> zone_label;
164+
// For generated column, just need a placeholder, and TiDB will fill this column.
165+
std::vector<std::tuple<UInt64, String, DataTypePtr>> generated_column_infos;
164166
};
165167
} // namespace DB

dbms/src/Storages/StorageDisaggregatedRemote.cpp

Lines changed: 60 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,8 @@ BlockInputStreams StorageDisaggregated::readThroughS3(const Context & db_context
115115
num_streams,
116116
pipeline,
117117
scan_context);
118+
// handle generated column if necessary.
119+
executeGeneratedColumnPlaceholder(generated_column_infos, log, pipeline);
118120

119121
NamesAndTypes source_columns;
120122
source_columns.reserve(table_scan.getColumnSize());
@@ -149,6 +151,8 @@ void StorageDisaggregated::readThroughS3(
149151
buildReadTaskWithBackoff(db_context, scan_context),
150152
num_streams,
151153
scan_context);
154+
// handle generated column if necessary.
155+
executeGeneratedColumnPlaceholder(exec_context, group_builder, generated_column_infos, log);
152156

153157
NamesAndTypes source_columns;
154158
auto header = group_builder.getCurrentHeader();
@@ -609,13 +613,14 @@ std::tuple<DM::RSOperatorPtr, DM::ColumnRangePtr> StorageDisaggregated::buildRSO
609613
return {rs_operator, column_range};
610614
}
611615

612-
std::variant<DM::Remote::RNWorkersPtr, DM::SegmentReadTaskPoolPtr> StorageDisaggregated::packSegmentReadTasks(
613-
const Context & db_context,
614-
DM::SegmentReadTasks && read_tasks,
615-
const DM::ColumnDefinesPtr & column_defines,
616-
const DM::ScanContextPtr & scan_context,
617-
size_t num_streams,
618-
int extra_table_id_index)
616+
std::tuple<std::variant<DM::Remote::RNWorkersPtr, DM::SegmentReadTaskPoolPtr>, DM::ColumnDefinesPtr> StorageDisaggregated::
617+
packSegmentReadTasks(
618+
const Context & db_context,
619+
DM::SegmentReadTasks && read_tasks,
620+
const DM::ColumnDefinesPtr & column_defines,
621+
const DM::ScanContextPtr & scan_context,
622+
size_t num_streams,
623+
int extra_table_id_index)
619624
{
620625
const auto & executor_id = table_scan.getTableScanExecutorID();
621626

@@ -651,53 +656,61 @@ std::variant<DM::Remote::RNWorkersPtr, DM::SegmentReadTaskPoolPtr> StorageDisagg
651656
scan_context->read_mode = read_mode;
652657
const UInt64 start_ts = sender_target_mpp_task_id.gather_id.query_id.start_ts;
653658
const auto enable_read_thread = db_context.getSettingsRef().dt_enable_read_thread;
659+
const auto & final_columns_defines = push_down_executor && push_down_executor->extra_cast
660+
? push_down_executor->columns_after_cast
661+
: column_defines;
654662
RUNTIME_CHECK(num_streams > 0, num_streams);
655663
LOG_INFO(
656664
log,
657665
"packSegmentReadTasks: enable_read_thread={} read_mode={} is_fast_scan={} keep_order={} task_count={} "
658-
"num_streams={} column_defines={}",
666+
"num_streams={} column_defines={} final_columns_defines={}",
659667
enable_read_thread,
660668
magic_enum::enum_name(read_mode),
661669
table_scan.isFastScan(),
662670
table_scan.keepOrder(),
663671
read_tasks.size(),
664672
num_streams,
665-
*column_defines);
673+
*column_defines,
674+
*final_columns_defines);
666675

667676
if (enable_read_thread)
668677
{
669678
// Under disagg arch, now we use blocking IO to read data from cloud storage. So it require more active
670679
// segments to fully utilize the read threads.
671680
const size_t read_thread_num_active_seg = 10 * num_streams;
672-
return std::make_shared<DM::SegmentReadTaskPool>(
673-
extra_table_id_index,
674-
*column_defines,
675-
push_down_executor,
676-
start_ts,
677-
db_context.getSettingsRef().max_block_size,
678-
read_mode,
679-
std::move(read_tasks),
680-
/*after_segment_read*/ [](const DM::DMContextPtr &, const DM::SegmentPtr &) {},
681-
log->identifier(),
682-
/*enable_read_thread*/ true,
683-
num_streams,
684-
read_thread_num_active_seg,
685-
context.getDAGContext()->getKeyspaceID(),
686-
context.getDAGContext()->getResourceGroupName());
681+
return {
682+
std::make_shared<DM::SegmentReadTaskPool>(
683+
extra_table_id_index,
684+
*final_columns_defines,
685+
push_down_executor,
686+
start_ts,
687+
db_context.getSettingsRef().max_block_size,
688+
read_mode,
689+
std::move(read_tasks),
690+
/*after_segment_read*/ [](const DM::DMContextPtr &, const DM::SegmentPtr &) {},
691+
log->identifier(),
692+
/*enable_read_thread*/ true,
693+
num_streams,
694+
read_thread_num_active_seg,
695+
context.getDAGContext()->getKeyspaceID(),
696+
context.getDAGContext()->getResourceGroupName()),
697+
final_columns_defines};
687698
}
688699
else
689700
{
690-
return DM::Remote::RNWorkers::create(
691-
db_context,
692-
std::move(read_tasks),
693-
{
694-
.log = log->getChild(executor_id),
695-
.columns_to_read = column_defines,
696-
.start_ts = start_ts,
697-
.push_down_executor = push_down_executor,
698-
.read_mode = read_mode,
699-
},
700-
num_streams);
701+
return {
702+
DM::Remote::RNWorkers::create(
703+
db_context,
704+
std::move(read_tasks),
705+
{
706+
.log = log->getChild(executor_id),
707+
.columns_to_read = final_columns_defines,
708+
.start_ts = start_ts,
709+
.push_down_executor = push_down_executor,
710+
.read_mode = read_mode,
711+
},
712+
num_streams),
713+
final_columns_defines};
701714
}
702715
}
703716

@@ -739,8 +752,11 @@ void StorageDisaggregated::buildRemoteSegmentInputStreams(
739752
const DM::ScanContextPtr & scan_context)
740753
{
741754
// Build the input streams to read blocks from remote segments
742-
auto [column_defines, extra_table_id_index] = genColumnDefinesForDisaggregatedRead(table_scan);
743-
auto packed_read_tasks = packSegmentReadTasks(
755+
DM::ColumnDefinesPtr column_defines;
756+
int extra_table_id_index;
757+
std::tie(column_defines, extra_table_id_index, generated_column_infos)
758+
= genColumnDefinesForDisaggregatedRead(table_scan);
759+
auto [packed_read_tasks, final_column_defines] = packSegmentReadTasks(
744760
db_context,
745761
std::move(read_tasks),
746762
column_defines,
@@ -751,7 +767,7 @@ void StorageDisaggregated::buildRemoteSegmentInputStreams(
751767

752768
InputStreamBuilder builder{
753769
.tracing_id = log->identifier(),
754-
.columns_to_read = column_defines,
770+
.columns_to_read = final_column_defines,
755771
.extra_table_id_index = extra_table_id_index,
756772
};
757773
for (size_t stream_idx = 0; stream_idx < num_streams; ++stream_idx)
@@ -810,8 +826,11 @@ void StorageDisaggregated::buildRemoteSegmentSourceOps(
810826
const DM::ScanContextPtr & scan_context)
811827
{
812828
// Build the input streams to read blocks from remote segments
813-
auto [column_defines, extra_table_id_index] = genColumnDefinesForDisaggregatedRead(table_scan);
814-
auto packed_read_tasks = packSegmentReadTasks(
829+
DM::ColumnDefinesPtr column_defines;
830+
int extra_table_id_index;
831+
std::tie(column_defines, extra_table_id_index, generated_column_infos)
832+
= genColumnDefinesForDisaggregatedRead(table_scan);
833+
auto [packed_read_tasks, final_column_defines] = packSegmentReadTasks(
815834
db_context,
816835
std::move(read_tasks),
817836
column_defines,
@@ -821,7 +840,7 @@ void StorageDisaggregated::buildRemoteSegmentSourceOps(
821840

822841
SourceOpBuilder builder{
823842
.tracing_id = log->identifier(),
824-
.column_defines = column_defines,
843+
.column_defines = final_column_defines,
825844
.extra_table_id_index = extra_table_id_index,
826845
.exec_context = exec_context,
827846
};

tests/docker/next-gen-config/tidb.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ disaggregated-tiflash = true
1919
# Now tests are ran on the SYSTEM keyspace tidb.
2020
keyspace-name = "SYSTEM"
2121

22+
tikv-worker-url = "tikv-worker0:19000"
23+
2224
enable-telemetry = false
2325
temp-dir = "/data/tmp"
2426
[performance]
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
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+
# object storage for next-gen
16+
[dfs]
17+
prefix = "tikv"
18+
s3-endpoint = "http://minio0:9000"
19+
s3-key-id = "minioadmin"
20+
s3-secret-key = "minioadmin"
21+
s3-bucket = "tiflash-test"
22+
s3-region = "local"
23+
24+
[schema-manager]
25+
dir = "/data/schemas"
26+
enabled = true
27+
keyspace-refresh-interval = "10s"
28+
schema-refresh-threshold = 1

tests/docker/next-gen-config/tikv.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ reserve-space = "0"
1818
# Enable keyspace and ttl for next-gen
1919
api-version = 2
2020
enable-ttl = true
21+
low-space-threshold = 0
2122

2223
[raftstore]
2324
capacity = "100GB"

tests/docker/next-gen-yaml/cluster.yaml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,19 @@ services:
4949
- "pd0"
5050
- "minio0"
5151
restart: on-failure
52+
tikv-worker0:
53+
image: ${TIKV_IMAGE:-us-docker.pkg.dev/pingcap-testing-account/hub/tikv/tikv/image:dedicated-next-gen}
54+
security_opt:
55+
- seccomp:unconfined
56+
volumes:
57+
- ./next-gen-config/tikv-worker.toml:/tikv-worker.toml:ro
58+
- ./data/tikv-worker0:/data
59+
- ./log/tikv-worker0:/log
60+
entrypoint: /tikv-worker
61+
command: --addr=0.0.0.0:19000 --pd-endpoints=pd0:2379 --config=/tikv-worker.toml --data-dir=/data --log-file=/log/tikv-worker.log
62+
depends_on:
63+
- "pd0"
64+
restart: on-failure
5265
tidb0:
5366
image: ${TIDB_IMAGE:-us-docker.pkg.dev/pingcap-testing-account/hub/pingcap/tidb/images/tidb-server:master-next-gen}
5467
security_opt:

tests/fullstack-test-next-gen/run.sh

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,15 @@ if [[ -n "$ENABLE_NEXT_GEN" && "$ENABLE_NEXT_GEN" != "false" && "$ENABLE_NEXT_GE
6969
ENV_ARGS="ENABLE_NEXT_GEN=true verbose=${verbose} "
7070
# most failpoints are expected to be set on the compute layer, use tiflash-cn0 to run tests
7171
${COMPOSE} -f next-gen-cluster.yaml -f "${DISAGG_TIFLASH_YAML}" exec -T tiflash-cn0 bash -c "cd /tests ; ${ENV_ARGS} ./run-test.sh fullstack-test/sample.test"
72-
${COMPOSE} -f next-gen-cluster.yaml -f "${DISAGG_TIFLASH_YAML}" exec -T tiflash-cn0 bash -c "cd /tests ; ${ENV_ARGS} ./run-test.sh fullstack-test-index/vector"
72+
${COMPOSE} -f next-gen-cluster.yaml -f "${DISAGG_TIFLASH_YAML}" exec -T tiflash-cn0 bash -c "cd /tests ; ${ENV_ARGS} ./run-test.sh fullstack-test-index"
7373
${COMPOSE} -f next-gen-cluster.yaml -f "${DISAGG_TIFLASH_YAML}" exec -T tiflash-cn0 bash -c "cd /tests ; ${ENV_ARGS} ./run-test.sh fullstack-test-next-gen/placement"
7474
${COMPOSE} -f next-gen-cluster.yaml -f "${DISAGG_TIFLASH_YAML}" exec -T tiflash-cn0 bash -c "cd /tests ; ${ENV_ARGS} ./run-test.sh fullstack-test2/clustered_index"
7575
${COMPOSE} -f next-gen-cluster.yaml -f "${DISAGG_TIFLASH_YAML}" exec -T tiflash-cn0 bash -c "cd /tests ; ${ENV_ARGS} ./run-test.sh fullstack-test2/dml"
7676
${COMPOSE} -f next-gen-cluster.yaml -f "${DISAGG_TIFLASH_YAML}" exec -T tiflash-cn0 bash -c "cd /tests ; ${ENV_ARGS} ./run-test.sh fullstack-test2/variables"
7777
${COMPOSE} -f next-gen-cluster.yaml -f "${DISAGG_TIFLASH_YAML}" exec -T tiflash-cn0 bash -c "cd /tests ; ${ENV_ARGS} ./run-test.sh fullstack-test2/mpp"
78-
# TODO: enable the following tests after they are fixed. And maybe we need to split them into parallel pipelines because they take too long to run.
79-
#${COMPOSE} -f next-gen-cluster.yaml -f "${DISAGG_TIFLASH_YAML}" exec -T tiflash-cn0 bash -c "cd /tests ; ${ENV_ARGS} ./run-test.sh fullstack-test/expr"
80-
#${COMPOSE} -f next-gen-cluster.yaml -f "${DISAGG_TIFLASH_YAML}" exec -T tiflash-cn0 bash -c "cd /tests ; ${ENV_ARGS} ./run-test.sh fullstack-test/mpp"
78+
# maybe we need to split them into parallel pipelines because they take too long to run.
79+
${COMPOSE} -f next-gen-cluster.yaml -f "${DISAGG_TIFLASH_YAML}" exec -T tiflash-cn0 bash -c "cd /tests ; ${ENV_ARGS} ./run-test.sh fullstack-test/expr"
80+
${COMPOSE} -f next-gen-cluster.yaml -f "${DISAGG_TIFLASH_YAML}" exec -T tiflash-cn0 bash -c "cd /tests ; ${ENV_ARGS} ./run-test.sh fullstack-test/mpp"
8181
${COMPOSE} -f next-gen-cluster.yaml -f "${DISAGG_TIFLASH_YAML}" down
8282
clean_data_log
8383

tests/fullstack-test/mpp/extra_physical_table_column.test

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,6 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
# line 27: Pessimistic lock response corrupted
16-
#SKIP_FOR_NEXT_GEN
17-
1815
# Preparation.
1916
=> DBGInvoke __init_fail_point()
2017

0 commit comments

Comments
 (0)