Skip to content

Commit c613cbf

Browse files
author
Daniel Q. Kim
committed
Fix Nullable(X)->Y cross-type ALTER MODIFY COLUMN
PR ClickHouse#84770 generates _CAST(ifNull(col, default), TargetType). When source and target base types differ (e.g. Nullable(UInt8) -> String), ifNull throws NO_COMMON_TYPE because getLeastSupertype({UInt8, String}) fails. Fix: swap to ifNull(_CAST(col, Nullable(TargetType)), _CAST(default, TargetType)). Ref: ClickHouse#84770, ClickHouse#5985 Signed-off-by: Daniel Q. Kim <daniel.kim@altinity.com>
1 parent c63afbe commit c613cbf

File tree

3 files changed

+31
-3
lines changed

3 files changed

+31
-3
lines changed

src/Interpreters/inplaceBlockConversions.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,10 +166,18 @@ ASTPtr convertRequiredExpressions(Block & block, const NamesAndTypesList & requi
166166
"Please specify `DEFAULT` expression in ALTER MODIFY COLUMN statement",
167167
required_column.name, column_in_block.type->getName(), required_column.type->getName());
168168

169-
auto convert_func = makeASTFunction("_CAST",
170-
makeASTFunction("ifNull", std::make_shared<ASTIdentifier>(required_column.name), default_value),
169+
/// Cast the nullable column to Nullable(TargetType) first, then strip NULLs with ifNull.
170+
/// This handles cross-type conversion (e.g. Nullable(UInt8) -> String) correctly,
171+
/// because _CAST(Nullable(UInt8), 'Nullable(String)') -> Nullable(String),
172+
/// and ifNull(Nullable(String), String) trivially resolves to String.
173+
auto nullable_target_type_name = "Nullable(" + required_column.type->getName() + ")";
174+
auto cast_col = makeASTFunction("_CAST",
175+
std::make_shared<ASTIdentifier>(required_column.name),
176+
std::make_shared<ASTLiteral>(nullable_target_type_name));
177+
auto cast_default = makeASTFunction("_CAST",
178+
default_value,
171179
std::make_shared<ASTLiteral>(required_column.type->getName()));
172-
180+
auto convert_func = makeASTFunction("ifNull", std::move(cast_col), std::move(cast_default));
173181
conversion_expr_list->children.emplace_back(setAlias(convert_func, required_column.name));
174182
continue;
175183
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
\N a
2+
42 b
3+
a
4+
42 b
5+
a
6+
42 b
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
-- Tags: no-random-settings, no-random-merge-tree-settings
2+
3+
-- Test cross-type Nullable(X) -> Y conversion (e.g. Nullable(UInt8) -> String)
4+
-- Follow-up to PR #84770
5+
6+
DROP TABLE IF EXISTS nullable_cross_type_test;
7+
CREATE TABLE nullable_cross_type_test (x Nullable(UInt8), y String) ORDER BY tuple();
8+
INSERT INTO nullable_cross_type_test VALUES (NULL, 'a'), (42, 'b');
9+
SELECT * FROM nullable_cross_type_test ORDER BY y;
10+
ALTER TABLE nullable_cross_type_test MODIFY COLUMN x String DEFAULT '';
11+
SELECT * FROM nullable_cross_type_test ORDER BY y;
12+
OPTIMIZE TABLE nullable_cross_type_test FINAL;
13+
SELECT * FROM nullable_cross_type_test ORDER BY y;
14+
DROP TABLE nullable_cross_type_test;

0 commit comments

Comments
 (0)