Skip to content

Commit 83ee9a6

Browse files
alesapinalesapin
authored
Merge pull request ClickHouse#63353 from ClickHouse/fix_unexpected_return_type_from
Fix logical error during SELECT query after ALTER in rare case
2 parents 78af18a + ff810ce commit 83ee9a6

7 files changed

+193
-11
lines changed

‎src/Storages/MergeTree/IMergeTreeReader.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,8 @@ void IMergeTreeReader::performRequiredConversions(Columns & res_columns) const
264264
/// Move columns from block.
265265
name_and_type = requested_columns.begin();
266266
for (size_t pos = 0; pos < num_columns; ++pos, ++name_and_type)
267-
res_columns[pos] = std::move(copy_block.getByName(name_and_type->name).column);
267+
if (copy_block.has(name_and_type->name))
268+
res_columns[pos] = std::move(copy_block.getByName(name_and_type->name).column);
268269
}
269270
catch (Exception & e)
270271
{

‎src/Storages/MergeTree/MergeTreeRangeReader.cpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,6 +1007,10 @@ MergeTreeRangeReader::ReadResult MergeTreeRangeReader::read(size_t max_rows, Mar
10071007
filterColumns(columns, read_result.final_filter);
10081008
}
10091009

1010+
/// If columns not empty, then apply on-fly alter conversions if any required
1011+
if (!prewhere_info || prewhere_info->perform_alter_conversions)
1012+
merge_tree_reader->performRequiredConversions(columns);
1013+
10101014
/// If some columns absent in part, then evaluate default values
10111015
if (should_evaluate_missing_defaults)
10121016
{
@@ -1017,10 +1021,6 @@ MergeTreeRangeReader::ReadResult MergeTreeRangeReader::read(size_t max_rows, Mar
10171021
addDummyColumnWithRowCount(additional_columns, read_result.num_rows);
10181022
merge_tree_reader->evaluateMissingDefaults(additional_columns, columns);
10191023
}
1020-
1021-
/// If columns not empty, then apply on-fly alter conversions if any required
1022-
if (!prewhere_info || prewhere_info->perform_alter_conversions)
1023-
merge_tree_reader->performRequiredConversions(columns);
10241024
}
10251025

10261026
read_result.columns.reserve(read_result.columns.size() + columns.size());
@@ -1046,14 +1046,14 @@ MergeTreeRangeReader::ReadResult MergeTreeRangeReader::read(size_t max_rows, Mar
10461046
bool should_evaluate_missing_defaults;
10471047
merge_tree_reader->fillMissingColumns(columns, should_evaluate_missing_defaults, read_result.num_rows);
10481048

1049-
/// If some columns absent in part, then evaluate default values
1050-
if (should_evaluate_missing_defaults)
1051-
merge_tree_reader->evaluateMissingDefaults({}, columns);
1052-
10531049
/// If result not empty, then apply on-fly alter conversions if any required
10541050
if (!prewhere_info || prewhere_info->perform_alter_conversions)
10551051
merge_tree_reader->performRequiredConversions(columns);
10561052

1053+
/// If some columns absent in part, then evaluate default values
1054+
if (should_evaluate_missing_defaults)
1055+
merge_tree_reader->evaluateMissingDefaults({}, columns);
1056+
10571057
for (size_t i = 0; i < columns.size(); ++i)
10581058
read_result.columns[i] = std::move(columns[i]);
10591059
}

‎src/Storages/MergeTree/MergeTreeSequentialSource.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,11 +253,11 @@ try
253253
bool should_evaluate_missing_defaults = false;
254254
reader->fillMissingColumns(columns, should_evaluate_missing_defaults, rows_read);
255255

256+
reader->performRequiredConversions(columns);
257+
256258
if (should_evaluate_missing_defaults)
257259
reader->evaluateMissingDefaults({}, columns);
258260

259-
reader->performRequiredConversions(columns);
260-
261261
/// Reorder columns and fill result block.
262262
size_t num_columns = sample.size();
263263
Columns res_columns;

‎tests/queries/0_stateless/03144_alter_column_and_read.reference‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
0 0_42
2+
1 1_42
3+
2 2_42
4+
3 3_42
5+
4 4_42
6+
5 5_42
7+
6 6_42
8+
7 7_42
9+
8 8_42
10+
9 9_42

‎tests/queries/0_stateless/03144_alter_column_and_read.sql‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
drop table if exists tab;
2+
create table tab (x UInt32) engine = MergeTree order by tuple();
3+
4+
insert into tab select number from numbers(10);
5+
6+
set alter_sync = 0;
7+
alter table tab update x = x + sleepEachRow(0.1) where 1;
8+
alter table tab modify column x String;
9+
alter table tab add column y String default x || '_42';
10+
11+
select x, y from tab order by x;

‎tests/queries/0_stateless/03144_parallel_alter_add_drop_column_zookeeper_on_steroids.reference‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
Starting alters
2+
Finishing alters
3+
Equal number of columns
4+
Replication did not hang: synced all replicas of concurrent_alter_add_drop_steroids_
5+
Consistency: 1
6+
0
7+
0
8+
0
9+
0
10+
0
11+
0

‎tests/queries/0_stateless/03144_parallel_alter_add_drop_column_zookeeper_on_steroids.sh‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
#!/usr/bin/env bash
2+
# Tags: zookeeper, no-parallel, no-fasttest
3+
4+
CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
5+
# shellcheck source=../shell_config.sh
6+
. "$CURDIR"/../shell_config.sh
7+
# shellcheck source=./replication.lib
8+
. "$CURDIR"/replication.lib
9+
10+
REPLICAS=3
11+
12+
for i in $(seq $REPLICAS); do
13+
$CLICKHOUSE_CLIENT --query "DROP TABLE IF EXISTS concurrent_alter_add_drop_steroids_$i"
14+
done
15+
16+
17+
for i in $(seq $REPLICAS); do
18+
$CLICKHOUSE_CLIENT --query "CREATE TABLE concurrent_alter_add_drop_steroids_$i (key UInt64, value0 UInt8) ENGINE = ReplicatedMergeTree('/clickhouse/tables/$CLICKHOUSE_TEST_ZOOKEEPER_PREFIX/concurrent_alter_add_drop_steroids_column', '$i') ORDER BY key SETTINGS max_replicated_mutations_in_queue = 1000, number_of_free_entries_in_pool_to_execute_mutation = 0, max_replicated_merges_in_queue = 1000, index_granularity = 8192, index_granularity_bytes = '10Mi'"
19+
done
20+
21+
$CLICKHOUSE_CLIENT --query "INSERT INTO concurrent_alter_add_drop_steroids_1 SELECT number, number + 10 from numbers(100000)"
22+
23+
for i in $(seq $REPLICAS); do
24+
$CLICKHOUSE_CLIENT --query "SYSTEM SYNC REPLICA concurrent_alter_add_drop_steroids_$i"
25+
done
26+
27+
28+
function alter_thread()
29+
{
30+
while true; do
31+
REPLICA=$(($RANDOM % 3 + 1))
32+
ADD=$(($RANDOM % 5 + 1))
33+
$CLICKHOUSE_CLIENT --query "ALTER TABLE concurrent_alter_add_drop_steroids_$REPLICA ADD COLUMN value$ADD UInt32 DEFAULT 42 SETTINGS replication_alter_partitions_sync=0"; # additionaly we don't wait anything for more heavy concurrency
34+
DROP=$(($RANDOM % 5 + 1))
35+
$CLICKHOUSE_CLIENT --query "ALTER TABLE concurrent_alter_add_drop_steroids_$REPLICA DROP COLUMN value$DROP SETTINGS replication_alter_partitions_sync=0"; # additionaly we don't wait anything for more heavy concurrency
36+
sleep 0.$RANDOM
37+
done
38+
}
39+
40+
function alter_thread_1()
41+
{
42+
while true; do
43+
REPLICA=$(($RANDOM % 3 + 1))
44+
${CLICKHOUSE_CLIENT} --query "ALTER TABLE concurrent_alter_add_drop_steroids_1 MODIFY COLUMN value0 String SETTINGS mutations_sync = 0"
45+
sleep 1.$RANDOM
46+
${CLICKHOUSE_CLIENT} --query "ALTER TABLE concurrent_alter_add_drop_steroids_1 MODIFY COLUMN value0 UInt8 SETTINGS mutations_sync = 0"
47+
sleep 1.$RANDOM
48+
done
49+
50+
}
51+
52+
function optimize_thread()
53+
{
54+
while true; do
55+
REPLICA=$(($RANDOM % 3 + 1))
56+
$CLICKHOUSE_CLIENT --query "OPTIMIZE TABLE concurrent_alter_add_drop_steroids_$REPLICA FINAL SETTINGS replication_alter_partitions_sync=0";
57+
sleep 0.$RANDOM
58+
done
59+
}
60+
61+
function insert_thread()
62+
{
63+
while true; do
64+
REPLICA=$(($RANDOM % 3 + 1))
65+
$CLICKHOUSE_CLIENT --query "INSERT INTO concurrent_alter_add_drop_steroids_$REPLICA VALUES($RANDOM, 7)"
66+
sleep 0.$RANDOM
67+
done
68+
}
69+
70+
function select_thread()
71+
{
72+
while true; do
73+
REPLICA=$(($RANDOM % 3 + 1))
74+
$CLICKHOUSE_CLIENT --query "SELECT * FROM merge(currentDatabase(), 'concurrent_alter_add_drop_steroids_') FORMAT Null"
75+
sleep 0.$RANDOM
76+
done
77+
}
78+
79+
80+
echo "Starting alters"
81+
export -f alter_thread;
82+
export -f alter_thread_1;
83+
export -f select_thread;
84+
export -f optimize_thread;
85+
export -f insert_thread;
86+
87+
88+
TIMEOUT=30
89+
90+
# Sometimes we detach and attach tables
91+
timeout $TIMEOUT bash -c alter_thread 2> /dev/null &
92+
timeout $TIMEOUT bash -c alter_thread 2> /dev/null &
93+
timeout $TIMEOUT bash -c alter_thread 2> /dev/null &
94+
95+
timeout $TIMEOUT bash -c alter_thread_1 2> /dev/null &
96+
timeout $TIMEOUT bash -c alter_thread_1 2> /dev/null &
97+
timeout $TIMEOUT bash -c alter_thread_1 2> /dev/null &
98+
99+
timeout $TIMEOUT bash -c select_thread 2> /dev/null &
100+
timeout $TIMEOUT bash -c select_thread 2> /dev/null &
101+
timeout $TIMEOUT bash -c select_thread 2> /dev/null &
102+
103+
timeout $TIMEOUT bash -c optimize_thread 2> /dev/null &
104+
timeout $TIMEOUT bash -c optimize_thread 2> /dev/null &
105+
timeout $TIMEOUT bash -c optimize_thread 2> /dev/null &
106+
107+
timeout $TIMEOUT bash -c insert_thread 2> /dev/null &
108+
timeout $TIMEOUT bash -c insert_thread 2> /dev/null &
109+
timeout $TIMEOUT bash -c insert_thread 2> /dev/null &
110+
timeout $TIMEOUT bash -c insert_thread 2> /dev/null &
111+
timeout $TIMEOUT bash -c insert_thread 2> /dev/null &
112+
113+
wait
114+
115+
echo "Finishing alters"
116+
117+
columns1=$($CLICKHOUSE_CLIENT --query "select count() from system.columns where table='concurrent_alter_add_drop_steroids_1' and database='$CLICKHOUSE_DATABASE'" 2> /dev/null)
118+
columns2=$($CLICKHOUSE_CLIENT --query "select count() from system.columns where table='concurrent_alter_add_drop_steroids_2' and database='$CLICKHOUSE_DATABASE'" 2> /dev/null)
119+
columns3=$($CLICKHOUSE_CLIENT --query "select count() from system.columns where table='concurrent_alter_add_drop_steroids_3' and database='$CLICKHOUSE_DATABASE'" 2> /dev/null)
120+
121+
while [ "$columns1" != "$columns2" ] || [ "$columns2" != "$columns3" ]; do
122+
columns1=$($CLICKHOUSE_CLIENT --query "select count() from system.columns where table='concurrent_alter_add_drop_steroids_1' and database='$CLICKHOUSE_DATABASE'" 2> /dev/null)
123+
columns2=$($CLICKHOUSE_CLIENT --query "select count() from system.columns where table='concurrent_alter_add_drop_steroids_2' and database='$CLICKHOUSE_DATABASE'" 2> /dev/null)
124+
columns3=$($CLICKHOUSE_CLIENT --query "select count() from system.columns where table='concurrent_alter_add_drop_steroids_3' and database='$CLICKHOUSE_DATABASE'" 2> /dev/null)
125+
126+
sleep 1
127+
done
128+
129+
echo "Equal number of columns"
130+
131+
# This alter will finish all previous, but replica 1 maybe still not up-to-date
132+
while [[ $(timeout 120 ${CLICKHOUSE_CLIENT} --query "ALTER TABLE concurrent_alter_add_drop_steroids_1 MODIFY COLUMN value0 String SETTINGS replication_alter_partitions_sync=2" 2>&1) ]]; do
133+
sleep 1
134+
done
135+
136+
check_replication_consistency "concurrent_alter_add_drop_steroids_" "count(), sum(key), sum(cityHash64(value0))"
137+
138+
for i in $(seq $REPLICAS); do
139+
$CLICKHOUSE_CLIENT --query "SYSTEM SYNC REPLICA concurrent_alter_add_drop_steroids_$i"
140+
$CLICKHOUSE_CLIENT --query "SELECT COUNT() FROM system.mutations WHERE is_done = 0 and table = 'concurrent_alter_add_drop_steroids_$i'"
141+
$CLICKHOUSE_CLIENT --query "SELECT * FROM system.mutations WHERE is_done = 0 and table = 'concurrent_alter_add_drop_steroids_$i'"
142+
$CLICKHOUSE_CLIENT --query "SELECT COUNT() FROM system.replication_queue WHERE table = 'concurrent_alter_add_drop_steroids_$i'"
143+
$CLICKHOUSE_CLIENT --query "SELECT * FROM system.replication_queue WHERE table = 'concurrent_alter_add_drop_steroids_$i' and (type = 'ALTER_METADATA' or type = 'MUTATE_PART')"
144+
145+
$CLICKHOUSE_CLIENT --query "DETACH TABLE concurrent_alter_add_drop_steroids_$i"
146+
$CLICKHOUSE_CLIENT --query "ATTACH TABLE concurrent_alter_add_drop_steroids_$i"
147+
148+
$CLICKHOUSE_CLIENT --query "DROP TABLE IF EXISTS concurrent_alter_add_drop_steroids_$i"
149+
done

0 commit comments

Comments
 (0)