25.8.14 Stable backport of #84770: Handle NULLs in ALTER MODIFY COLUMN

src/Interpreters/inplaceBlockConversions.cpp

@@ -21,13 +21,13 @@
 #include <DataTypes/DataTypeArray.h>
 #include <Storages/StorageInMemoryMetadata.h>
 
-
 namespace DB
 {
 
-namespace ErrorCode
+namespace ErrorCodes
 {
     extern const int LOGICAL_ERROR;
+    extern const int BAD_ARGUMENTS;
 }
 
 namespace
@@ -136,7 +136,7 @@ ASTPtr defaultRequiredExpressions(const Block & block, const NamesAndTypesList &
     return default_expr_list;
 }
 
-ASTPtr convertRequiredExpressions(Block & block, const NamesAndTypesList & required_columns)
+ASTPtr convertRequiredExpressions(Block & block, const NamesAndTypesList & required_columns, const ColumnDefaults & column_defaults, bool forbid_default_defaults)
 {
     ASTPtr conversion_expr_list = std::make_shared<ASTExpressionList>();
     for (const auto & required_column : required_columns)
@@ -148,6 +148,32 @@ ASTPtr convertRequiredExpressions(Block & block, const NamesAndTypesList & requi
         if (column_in_block.type->equals(*required_column.type))
             continue;
 
+        /// Converting a column from nullable to non-nullable may cause 'Cannot convert column' error when NULL values exist.
+        /// Users should specify DEFAULT expression in ALTER MODIFY COLUMN statement to replace NULL values.
+        if (isNullableOrLowCardinalityNullable(column_in_block.type) && !isNullableOrLowCardinalityNullable(required_column.type))
+        {
+            /// Before executing ALTER we explicitly check that user provided DEFAULT value to make it a conscious decision.
+            /// However, we may still need to use type's default value in some cases
+            /// (e.g. if a second ALTER removes the DEFAULT, but first is not completed).
+            ASTPtr default_value;
+            if (auto it = column_defaults.find(required_column.name); it != column_defaults.end())
+                default_value = it->second.expression;
+            else if (!forbid_default_defaults)
+                default_value = std::make_shared<ASTLiteral>(required_column.type->getDefault());
+            else
+                throw Exception(ErrorCodes::BAD_ARGUMENTS,
+                    "Cannot convert column '{}' from nullable type {} to non-nullable type {}. "
+                    "Please specify `DEFAULT` expression in ALTER MODIFY COLUMN statement",
+                    required_column.name, column_in_block.type->getName(), required_column.type->getName());
+
+            auto convert_func = makeASTFunction("_CAST",
+                makeASTFunction("ifNull", std::make_shared<ASTIdentifier>(required_column.name), default_value),
+                std::make_shared<ASTLiteral>(required_column.type->getName()));
+
+            conversion_expr_list->children.emplace_back(setAlias(convert_func, required_column.name));
+            continue;
+        }
+
         auto cast_func = makeASTFunction(
             "_CAST", std::make_shared<ASTIdentifier>(required_column.name), std::make_shared<ASTLiteral>(required_column.type->getName()));
 
@@ -175,9 +201,9 @@ std::optional<ActionsDAG> createExpressions(
 
 }
 
-void performRequiredConversions(Block & block, const NamesAndTypesList & required_columns, ContextPtr context)
+void performRequiredConversions(Block & block, const NamesAndTypesList & required_columns, ContextPtr context, const ColumnDefaults & column_defaults, bool forbid_default_defaults)
 {
-    ASTPtr conversion_expr_list = convertRequiredExpressions(block, required_columns);
+    ASTPtr conversion_expr_list = convertRequiredExpressions(block, required_columns, column_defaults, forbid_default_defaults);
     if (conversion_expr_list->children.empty())
         return;
 

src/Interpreters/inplaceBlockConversions.h

@@ -3,6 +3,7 @@
 #include <Core/Names.h>
 #include <Interpreters/Context_fwd.h>
 #include <Common/COW.h>
+#include <Storages/ColumnDefault.h>
 
 #include <memory>
 
@@ -34,7 +35,8 @@ std::optional<ActionsDAG> evaluateMissingDefaults(
     bool null_as_default = false);
 
 /// Tries to convert columns in block to required_columns
-void performRequiredConversions(Block & block, const NamesAndTypesList & required_columns, ContextPtr context);
+void performRequiredConversions(Block & block, const NamesAndTypesList & required_columns, ContextPtr context,
+    const ColumnDefaults & column_defaults, bool forbid_default_defaults = false);
 
 void fillMissingColumns(
     Columns & res_columns,

src/Storages/AlterCommands.cpp

@@ -1327,7 +1327,7 @@ void AlterCommands::apply(StorageInMemoryMetadata & metadata, ContextPtr context
                 throw Exception(ErrorCodes::ALTER_OF_COLUMN_IS_FORBIDDEN, "Cannot ALTER column");
             /// Check if new metadata is convertible from old metadata for projection.
             Block old_projection_block = projection.sample_block;
-            performRequiredConversions(old_projection_block, new_projection.sample_block.getNamesAndTypesList(), context);
+            performRequiredConversions(old_projection_block, new_projection.sample_block.getNamesAndTypesList(), context, metadata_copy.getColumns().getDefaults());
             new_projections.add(std::move(new_projection));
         }
         catch (Exception & exception)

src/Storages/MergeTree/IMergeTreeReader.cpp

@@ -333,7 +333,7 @@ void IMergeTreeReader::performRequiredConversions(Columns & res_columns) const
             copy_block.insert({res_columns[pos], getColumnInPart(*name_and_type).type, name_and_type->name});
         }
 
-        DB::performRequiredConversions(copy_block, requested_columns, data_part_info_for_read->getContext());
+        DB::performRequiredConversions(copy_block, requested_columns, data_part_info_for_read->getContext(), storage_snapshot->metadata->getColumns().getDefaults());
 
         /// Move columns from block.
         name_and_type = requested_columns.begin();

src/Storages/MergeTree/MergeTreeData.cpp

@@ -4349,7 +4349,7 @@ void MergeTreeData::checkAlterIsPossible(const AlterCommands & commands, Context
     if (!columns_to_check_conversion.empty())
     {
         auto old_header = old_metadata.getSampleBlock();
-        performRequiredConversions(old_header, columns_to_check_conversion, local_context);
+        performRequiredConversions(old_header, columns_to_check_conversion, local_context, new_metadata.getColumns().getDefaults(), true);
     }
 
     if (old_metadata.hasSettingsChanges())

tests/queries/0_stateless/02662_sparse_columns_mutations_1.sql

@@ -30,7 +30,7 @@ ORDER BY name;
 
 SELECT countIf(s = 'foo'), arraySort(groupUniqArray(s)) FROM t_sparse_mutations_1;
 
-ALTER TABLE t_sparse_mutations_1 MODIFY COLUMN s String;
+ALTER TABLE t_sparse_mutations_1 MODIFY COLUMN s String DEFAULT '';
 
 SELECT name, type, serialization_kind FROM system.parts_columns
 WHERE database = currentDatabase() AND table = 't_sparse_mutations_1' AND column = 's' AND active

tests/queries/0_stateless/03575_modify_column_null_to_default.reference

@@ -0,0 +1,38 @@
+-- { echoOn }
+
+SELECT * from nullable_test ORDER BY ALL;
+1	1	1	A
+\N	\N	\N	\N
+SYSTEM STOP MERGES nullable_test;
+ALTER TABLE nullable_test MODIFY COLUMN my_int_nullable UInt64 SETTINGS mutations_sync = 0, alter_sync = 0; -- { serverError BAD_ARGUMENTS }
+ALTER TABLE nullable_test MODIFY COLUMN my_int_nullable UInt64 DEFAULT 42 SETTINGS mutations_sync = 0, alter_sync = 0;
+SELECT * from nullable_test ORDER BY ALL;
+1	1	1	A
+42	\N	\N	\N
+SYSTEM START MERGES nullable_test;
+SELECT * from nullable_test ORDER BY ALL;
+1	1	1	A
+42	\N	\N	\N
+OPTIMIZE TABLE nullable_test FINAL;
+SELECT * from nullable_test ORDER BY ALL;
+1	1	1	A
+42	\N	\N	\N
+ALTER TABLE nullable_test MODIFY COLUMN my_text_lc_nullable String DEFAULT 'empty';
+SELECT * from nullable_test ORDER BY ALL;
+1	1	1	A
+42	\N	\N	empty
+-- Previouly existing DEFAULT NULL does not allow to modify
+ALTER TABLE nullable_test MODIFY COLUMN my_int_nullable_with_default UInt64 SETTINGS mutations_sync = 0, alter_sync = 0; -- { serverError CANNOT_CONVERT_TYPE }
+SELECT * from nullable_test ORDER BY ALL;
+1	1	1	A
+42	\N	\N	empty
+ALTER TABLE nullable_test MODIFY COLUMN my_int_nullable_with_default UInt64 DEFAULT 43 SETTINGS mutations_sync = 0, alter_sync = 0;
+SELECT * from nullable_test ORDER BY ALL;
+1	1	1	A
+42	43	\N	empty
+-- But when we have DEFAULT which is non NULL we can keep it
+ALTER TABLE nullable_test MODIFY COLUMN my_int_nullable_with_default2 UInt64 SETTINGS mutations_sync = 0, alter_sync = 0;
+SELECT * from nullable_test ORDER BY ALL;
+1	1	1	A
+42	43	11	empty
+DROP TABLE IF EXISTS nullable_test;

tests/queries/0_stateless/03575_modify_column_null_to_default.sql

@@ -0,0 +1,46 @@
+DROP TABLE IF EXISTS nullable_test;
+
+CREATE TABLE nullable_test(
+    my_int_nullable Nullable(UInt32),
+    my_int_nullable_with_default Nullable(UInt32) DEFAULT NULL,
+    my_int_nullable_with_default2 Nullable(UInt32) DEFAULT 11,
+    my_text_lc_nullable LowCardinality(Nullable(String)),
+) ORDER BY tuple();
+
+INSERT INTO nullable_test VALUES (NULL, NULL, NULL, NULL), (1, 1, 1, 'A');
+
+-- { echoOn }
+
+SELECT * from nullable_test ORDER BY ALL;
+
+SYSTEM STOP MERGES nullable_test;
+
+ALTER TABLE nullable_test MODIFY COLUMN my_int_nullable UInt64 SETTINGS mutations_sync = 0, alter_sync = 0; -- { serverError BAD_ARGUMENTS }
+ALTER TABLE nullable_test MODIFY COLUMN my_int_nullable UInt64 DEFAULT 42 SETTINGS mutations_sync = 0, alter_sync = 0;
+
+SELECT * from nullable_test ORDER BY ALL;
+
+SYSTEM START MERGES nullable_test;
+
+SELECT * from nullable_test ORDER BY ALL;
+
+OPTIMIZE TABLE nullable_test FINAL;
+SELECT * from nullable_test ORDER BY ALL;
+
+ALTER TABLE nullable_test MODIFY COLUMN my_text_lc_nullable String DEFAULT 'empty';
+SELECT * from nullable_test ORDER BY ALL;
+
+-- Previouly existing DEFAULT NULL does not allow to modify
+ALTER TABLE nullable_test MODIFY COLUMN my_int_nullable_with_default UInt64 SETTINGS mutations_sync = 0, alter_sync = 0; -- { serverError CANNOT_CONVERT_TYPE }
+
+SELECT * from nullable_test ORDER BY ALL;
+
+ALTER TABLE nullable_test MODIFY COLUMN my_int_nullable_with_default UInt64 DEFAULT 43 SETTINGS mutations_sync = 0, alter_sync = 0;
+SELECT * from nullable_test ORDER BY ALL;
+
+-- But when we have DEFAULT which is non NULL we can keep it
+ALTER TABLE nullable_test MODIFY COLUMN my_int_nullable_with_default2 UInt64 SETTINGS mutations_sync = 0, alter_sync = 0;
+
+SELECT * from nullable_test ORDER BY ALL;
+
+DROP TABLE IF EXISTS nullable_test;