Skip to content

Commit 782b09b

Browse files
amosbirdamosbird
committed
Disallow adding _part_offset column
1 parent c54e4d2 commit 782b09b

File tree

4 files changed

+126
-3
lines changed

4 files changed

+126
-3
lines changed

‎src/Storages/MergeTree/MergeTreeData.cpp‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -913,6 +913,30 @@ void MergeTreeData::checkProperties(
913913
}
914914
}
915915

916+
String projection_with_parent_part_offset;
917+
for (const auto & projection : old_metadata.projections)
918+
{
919+
if (projection.with_parent_part_offset)
920+
{
921+
projection_with_parent_part_offset = projection.name;
922+
break;
923+
}
924+
}
925+
926+
if (!projection_with_parent_part_offset.empty())
927+
{
928+
for (const auto & col : new_metadata.columns)
929+
{
930+
if (col.name == "_part_offset" || col.name == "_part_index" || col.name == "_parent_part_offset")
931+
throw Exception(
932+
ErrorCodes::BAD_ARGUMENTS,
933+
"Cannot add column `{}` because normal projection {} references its parent `_part_offset` column. "
934+
"Columns named `_part_offset`, `_part_index`, or `_parent_part_offset` are not allowed in this case",
935+
col.name,
936+
projection_with_parent_part_offset);
937+
}
938+
}
939+
916940
for (const auto & col : new_metadata.columns)
917941
{
918942
if (!col.statistics.empty())

‎src/Storages/ProjectionsDescription.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ ProjectionDescription::getProjectionFromAST(const ASTPtr & definition_ast, const
222222
/// Prevent normal projection from storing parent part offset if the parent table defines `_parent_part_offset` or
223223
/// `_part_offset` as physical columns, which would cause a conflict. Parent table cannot defines `_part_index` as
224224
/// physical column either because it's used to build part offset mapping during merge.
225-
bool cannot_hold_parent_part_offset = columns.has("_part_index") || columns.has("_part_offset") || columns.has("_parent_part_offset");
225+
bool can_hold_parent_part_offset = !(columns.has("_part_index") || columns.has("_part_offset") || columns.has("_parent_part_offset"));
226226

227227
StoragePtr storage = std::make_shared<StorageProjectionSource>(columns);
228228
InterpreterSelectQuery select(
@@ -247,7 +247,7 @@ ProjectionDescription::getProjectionFromAST(const ASTPtr & definition_ast, const
247247
if (select.hasAggregation())
248248
{
249249
/// Aggregate projections cannot hold parent part offset.
250-
cannot_hold_parent_part_offset = true;
250+
can_hold_parent_part_offset = false;
251251

252252
if (query.orderBy())
253253
throw Exception(ErrorCodes::ILLEGAL_PROJECTION, "When aggregation is used in projection, ORDER BY cannot be specified");
@@ -294,7 +294,7 @@ ProjectionDescription::getProjectionFromAST(const ASTPtr & definition_ast, const
294294
}
295295

296296
/// Rename parent _part_offset to _parent_part_offset column
297-
if (!cannot_hold_parent_part_offset && result.sample_block.has("_part_offset"))
297+
if (can_hold_parent_part_offset && result.sample_block.has("_part_offset"))
298298
{
299299
auto new_column = result.sample_block.getByName("_part_offset");
300300
new_column.name = "_parent_part_offset";

‎tests/queries/0_stateless/03401_normal_projection_with_part_offset.reference‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,50 @@ ALTER TABLE test ADD PROJECTION p2 (SELECT a, b, _part_offset ORDER BY b);
3030
ALTER TABLE test MATERIALIZE PROJECTION p2 SETTINGS mutations_sync = 2;
3131
SELECT sum(l._part_offset = r._parent_part_offset) FROM test l JOIN mergeTreeProjection(currentDatabase(), test, p2) r USING (a) SETTINGS enable_analyzer = 1;
3232
1080000
33+
-- Cannot add physical _part_offset, _part_index and _parent_part_offset when there exists projection that refers to its parent `_part_offset`.
34+
ALTER TABLE test ADD COLUMN _part_offset int; -- { serverError BAD_ARGUMENTS }
35+
ALTER TABLE test ADD COLUMN _part_index int; -- { serverError BAD_ARGUMENTS }
36+
ALTER TABLE test ADD COLUMN _parent_part_offset int; -- { serverError BAD_ARGUMENTS }
37+
DROP TABLE test;
38+
CREATE TABLE test
39+
(
40+
`a` Int32,
41+
`b` Int32,
42+
`_part_offset` Int32,
43+
PROJECTION p
44+
(
45+
SELECT
46+
a,
47+
b,
48+
_part_offset
49+
ORDER BY b
50+
)
51+
)
52+
ENGINE = MergeTree
53+
ORDER BY a;
54+
-- This works because now projection will refer to the parent's physical `_part_offset`
55+
ALTER TABLE test ADD COLUMN _part_index int;
56+
ALTER TABLE test ADD COLUMN _parent_part_offset int;
57+
DROP TABLE test;
58+
CREATE TABLE test
59+
(
60+
`a` Int32,
61+
`b` Int32,
62+
PROJECTION p
63+
(
64+
SELECT
65+
a,
66+
b,
67+
_part_offset
68+
ORDER BY b
69+
)
70+
)
71+
ENGINE = MergeTree
72+
ORDER BY a
73+
SETTINGS index_granularity = 1;
74+
INSERT INTO test SELECT number, 10 - number FROM numbers(5);
75+
-- Projection analysis should work
76+
SELECT _part_offset FROM test WHERE b = 8
77+
SETTINGS query_plan_enable_optimizations = 1, optimize_use_projections = 1, force_optimize_projection = 1;
78+
2
3379
DROP TABLE test;

‎tests/queries/0_stateless/03401_normal_projection_with_part_offset.sql‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,57 @@ ALTER TABLE test ADD PROJECTION p2 (SELECT a, b, _part_offset ORDER BY b);
3232
ALTER TABLE test MATERIALIZE PROJECTION p2 SETTINGS mutations_sync = 2;
3333
SELECT sum(l._part_offset = r._parent_part_offset) FROM test l JOIN mergeTreeProjection(currentDatabase(), test, p2) r USING (a) SETTINGS enable_analyzer = 1;
3434

35+
-- Cannot add physical _part_offset, _part_index and _parent_part_offset when there exists projection that refers to its parent `_part_offset`.
36+
ALTER TABLE test ADD COLUMN _part_offset int; -- { serverError BAD_ARGUMENTS }
37+
ALTER TABLE test ADD COLUMN _part_index int; -- { serverError BAD_ARGUMENTS }
38+
ALTER TABLE test ADD COLUMN _parent_part_offset int; -- { serverError BAD_ARGUMENTS }
39+
40+
DROP TABLE test;
41+
42+
CREATE TABLE test
43+
(
44+
`a` Int32,
45+
`b` Int32,
46+
`_part_offset` Int32,
47+
PROJECTION p
48+
(
49+
SELECT
50+
a,
51+
b,
52+
_part_offset
53+
ORDER BY b
54+
)
55+
)
56+
ENGINE = MergeTree
57+
ORDER BY a;
58+
59+
-- This works because now projection will refer to the parent's physical `_part_offset`
60+
ALTER TABLE test ADD COLUMN _part_index int;
61+
ALTER TABLE test ADD COLUMN _parent_part_offset int;
62+
63+
DROP TABLE test;
64+
65+
CREATE TABLE test
66+
(
67+
`a` Int32,
68+
`b` Int32,
69+
PROJECTION p
70+
(
71+
SELECT
72+
a,
73+
b,
74+
_part_offset
75+
ORDER BY b
76+
)
77+
)
78+
ENGINE = MergeTree
79+
ORDER BY a
80+
SETTINGS index_granularity = 1;
81+
82+
INSERT INTO test SELECT number, 10 - number FROM numbers(5);
83+
84+
-- Projection analysis should work
85+
SELECT _part_offset FROM test WHERE b = 8
86+
SETTINGS query_plan_enable_optimizations = 1, optimize_use_projections = 1, force_optimize_projection = 1;
87+
3588
DROP TABLE test;

0 commit comments

Comments
 (0)