Merge a3ae00adb8 into f4038e3153

src/Databases/DatabaseMemory.cpp

@@ -151,7 +151,7 @@ void DatabaseMemory::alterTable(ContextPtr local_context, const StorageID & tabl
     if (it == create_queries.end() || !it->second)
         throw Exception(ErrorCodes::UNKNOWN_TABLE, "Cannot alter: There is no metadata of table {}", table_id.getNameForLogs());
 
-    applyMetadataChangesToCreateQuery(it->second, metadata);
+    applyMetadataChangesToCreateQuery(it->second, metadata, local_context);
 
     /// The create query of the table has been just changed, we need to update dependencies too.
     auto ref_dependencies = getDependenciesFromCreateQuery(local_context->getGlobalContext(), table_id.getQualifiedName(), it->second, local_context->getCurrentDatabase());
@@ -204,7 +204,7 @@ std::vector<std::pair<ASTPtr, StoragePtr>> DatabaseMemory::getTablesForBackup(co
         }
 
         chassert(storage);
-        storage->adjustCreateQueryForBackup(create_table_query);
+        storage->adjustCreateQueryForBackup(create_table_query, local_context);
         res.emplace_back(create_table_query, storage);
     }
 

src/Databases/DatabaseOnDisk.cpp

@@ -115,7 +115,7 @@ std::pair<String, StoragePtr> createTableFromAST(
         else
         {
             columns = InterpreterCreateQuery::getColumnsDescription(*ast_create_query.columns_list->columns, context, mode);
-            constraints = InterpreterCreateQuery::getConstraintsDescription(ast_create_query.columns_list->constraints);
+            constraints = InterpreterCreateQuery::getConstraintsDescription(ast_create_query.columns_list->constraints, columns, context);
         }
     }
 

src/Databases/DatabaseOrdinary.cpp

@@ -552,7 +552,7 @@ void DatabaseOrdinary::alterTable(ContextPtr local_context, const StorageID & ta
         0,
         local_context->getSettingsRef().max_parser_depth, local_context->getSettingsRef().max_parser_backtracks);
 
-    applyMetadataChangesToCreateQuery(ast, metadata);
+    applyMetadataChangesToCreateQuery(ast, metadata, local_context);
 
     statement = getObjectDefinitionFromCreateQuery(ast);
     {

src/Databases/DatabaseReplicated.cpp

@@ -1752,7 +1752,7 @@ String DatabaseReplicated::readMetadataFile(const String & table_name) const
 
 
 std::vector<std::pair<ASTPtr, StoragePtr>>
-DatabaseReplicated::getTablesForBackup(const FilterByNameFunction & filter, const ContextPtr &) const
+DatabaseReplicated::getTablesForBackup(const FilterByNameFunction & filter, const ContextPtr & local_context) const
 {
     waitDatabaseStarted();
 
@@ -1778,7 +1778,7 @@ DatabaseReplicated::getTablesForBackup(const FilterByNameFunction & filter, cons
         {
             storage = DatabaseCatalog::instance().tryGetByUUID(create.uuid).second;
             if (storage)
-                storage->adjustCreateQueryForBackup(create_table_query);
+                storage->adjustCreateQueryForBackup(create_table_query, local_context);
         }
 
         /// `storage` is allowed to be null here. In this case it means that this storage exists on other replicas

src/Databases/DatabasesCommon.cpp

@@ -2,15 +2,20 @@
 
 #include <Backups/BackupEntriesCollector.h>
 #include <Backups/RestorerFromBackup.h>
+#include <Core/Settings.h>
 #include <Interpreters/Context.h>
 #include <Interpreters/DatabaseCatalog.h>
 #include <Interpreters/InterpreterCreateQuery.h>
+#include <Interpreters/TreeRewriter.h>
 #include <Parsers/ASTCreateQuery.h>
 #include <Parsers/ASTSelectWithUnionQuery.h>
 #include <Parsers/ParserCreateQuery.h>
 #include <Parsers/formatAST.h>
+#include <Parsers/parseQuery.h>
+#include <Storages/KeyDescription.h>
 #include <Storages/StorageDictionary.h>
 #include <Storages/StorageFactory.h>
+#include <Storages/TTLDescription.h>
 #include <Storages/Utils.h>
 #include <TableFunctions/TableFunctionFactory.h>
 #include <Common/CurrentMetrics.h>
@@ -18,7 +23,6 @@
 #include <Common/logger_useful.h>
 #include <Common/typeid_cast.h>
 
-
 namespace DB
 {
 
@@ -31,8 +35,65 @@ namespace ErrorCodes
     extern const int LOGICAL_ERROR;
     extern const int CANNOT_GET_CREATE_TABLE_QUERY;
 }
+namespace
+{
+void validateCreateQuery(const ASTCreateQuery & query, ContextPtr context)
+{
+    /// First validate that the query can be parsed
+    const auto serialized_query = serializeAST(query);
+    ParserCreateQuery parser;
+    ASTPtr new_query_raw = parseQuery(
+        parser,
+        serialized_query.data(),
+        serialized_query.data() + serialized_query.size(),
+        "after altering table ",
+        0,
+        context->getSettingsRef().max_parser_depth,
+        context->getSettingsRef().max_parser_backtracks);
+    const auto & new_query = new_query_raw->as<const ASTCreateQuery &>();
+    /// If there are no columns, then there is nothing much we can do
+    if (!new_query.columns_list || !new_query.columns_list->columns)
+        return;
 
-void applyMetadataChangesToCreateQuery(const ASTPtr & query, const StorageInMemoryMetadata & metadata)
+    const auto & columns = *new_query.columns_list;
+    /// Do some basic sanity checks. Let's enforce strict rules, just like on CREATE, because otherwise the default expressions might not be checked
+    const auto columns_desc
+        = InterpreterCreateQuery::getColumnsDescription(*columns.columns, context, LoadingStrictnessLevel::CREATE, false);
+
+    if (columns.indices)
+    {
+        for (const auto & child : columns.indices->children)
+            IndexDescription::getIndexFromAST(child, columns_desc, context);
+    }
+    if (columns.constraints)
+    {
+        InterpreterCreateQuery::getConstraintsDescription(columns.constraints, columns_desc, context);
+    }
+    if (columns.projections)
+    {
+        for (const auto & child : columns.projections->children)
+            ProjectionDescription::getProjectionFromAST(child, columns_desc, context);
+    }
+    if (!new_query.storage)
+        return;
+    const auto & storage = *new_query.storage;
+
+    std::optional<KeyDescription> primary_key;
+    /// First get the key description from order by, so if there is no primary key we will use that
+    if (storage.order_by)
+        primary_key = KeyDescription::getKeyFromAST(storage.order_by->ptr(), columns_desc, context);
+    if (storage.primary_key)
+        primary_key = KeyDescription::getKeyFromAST(storage.primary_key->ptr(), columns_desc, context);
+    if (storage.partition_by)
+        KeyDescription::getKeyFromAST(storage.partition_by->ptr(), columns_desc, context);
+    if (storage.sample_by)
+        KeyDescription::getKeyFromAST(storage.sample_by->ptr(), columns_desc, context);
+    if (storage.ttl_table && primary_key.has_value())
+        TTLTableDescription::getTTLForTableFromAST(storage.ttl_table->ptr(), columns_desc, context, *primary_key, true);
+}
+}
+
+void applyMetadataChangesToCreateQuery(const ASTPtr & query, const StorageInMemoryMetadata & metadata, ContextPtr context)
 {
     auto & ast_create_query = query->as<ASTCreateQuery &>();
 
@@ -115,6 +176,8 @@ void applyMetadataChangesToCreateQuery(const ASTPtr & query, const StorageInMemo
         ast_create_query.reset(ast_create_query.comment);
     else
         ast_create_query.set(ast_create_query.comment, std::make_shared<ASTLiteral>(metadata.comment));
+
+    validateCreateQuery(ast_create_query, context);
 }
 
 
@@ -426,7 +489,7 @@ std::vector<std::pair<ASTPtr, StoragePtr>> DatabaseWithOwnTablesBase::getTablesF
             create->setTable(it->name());
         }
 
-        storage->adjustCreateQueryForBackup(create_table_query);
+        storage->adjustCreateQueryForBackup(create_table_query, local_context);
         res.emplace_back(create_table_query, storage);
     }
 

src/Databases/DatabasesCommon.h

@@ -1,10 +1,9 @@
 #pragma once
 
-#include <base/types.h>
+#include <Databases/IDatabase.h>
 #include <Parsers/IAST.h>
 #include <Storages/IStorage_fwd.h>
-#include <Databases/IDatabase.h>
-#include <mutex>
+#include <base/types.h>
 
 
 /// General functionality for several different database engines.
@@ -12,15 +11,13 @@
 namespace DB
 {
 
-void applyMetadataChangesToCreateQuery(const ASTPtr & query, const StorageInMemoryMetadata & metadata);
+void applyMetadataChangesToCreateQuery(const ASTPtr & query, const StorageInMemoryMetadata & metadata, ContextPtr context);
 ASTPtr getCreateQueryFromStorage(const StoragePtr & storage, const ASTPtr & ast_storage, bool only_ordinary,
     uint32_t max_parser_depth, uint32_t max_parser_backtracks, bool throw_on_error);
 
 /// Cleans a CREATE QUERY from temporary flags like "IF NOT EXISTS", "OR REPLACE", "AS SELECT" (for non-views), etc.
 void cleanupObjectDefinitionFromTemporaryFlags(ASTCreateQuery & query);
 
-class Context;
-
 /// A base class for databases that manage their own list of tables.
 class DatabaseWithOwnTablesBase : public IDatabase, protected WithContext
 {

src/Interpreters/InterpreterCreateQuery.cpp

@@ -5,17 +5,18 @@
 #include <Access/AccessControl.h>
 #include <Access/User.h>
 
-#include <Common/Exception.h>
-#include <Common/StringUtils.h>
-#include <Common/escapeForFileName.h>
-#include <Common/typeid_cast.h>
-#include <Common/Macros.h>
-#include <Common/randomSeed.h>
-#include <Common/atomicRename.h>
-#include <Common/PoolId.h>
-#include <Common/logger_useful.h>
 #include <Core/Settings.h>
 #include <Parsers/ASTSetQuery.h>
+#include <Common/Exception.h>
+#include <Common/Macros.h>
+#include <Common/PoolId.h>
+#include <Common/StringUtils.h>
+#include <Common/atomicRename.h>
+#include <Common/escapeForFileName.h>
+#include <Common/logger_useful.h>
+#include <Common/randomSeed.h>
+#include <Common/typeid_cast.h>
+#include "Interpreters/TreeRewriter.h"
 
 #include <Core/Defines.h>
 #include <Core/SettingsEnums.h>
@@ -731,13 +732,18 @@ ColumnsDescription InterpreterCreateQuery::getColumnsDescription(
 }
 
 
-ConstraintsDescription InterpreterCreateQuery::getConstraintsDescription(const ASTExpressionList * constraints)
+ConstraintsDescription InterpreterCreateQuery::getConstraintsDescription(
+    const ASTExpressionList * constraints, const ColumnsDescription & columns, ContextPtr local_context)
 {
     ASTs constraints_data;
+    const auto column_names_and_types = columns.getAllPhysical();
     if (constraints)
         for (const auto & constraint : constraints->children)
+        {
+            auto clone = constraint->clone();
+            TreeRewriter(local_context).analyze(clone, column_names_and_types);
             constraints_data.push_back(constraint->clone());
-
+        }
     return ConstraintsDescription{constraints_data};
 }
 
@@ -800,7 +806,7 @@ InterpreterCreateQuery::TableProperties InterpreterCreateQuery::getTableProperti
                 properties.projections.add(std::move(projection));
             }
 
-        properties.constraints = getConstraintsDescription(create.columns_list->constraints);
+        properties.constraints = getConstraintsDescription(create.columns_list->constraints, properties.columns, getContext());
     }
     else if (!create.as_table.empty())
     {

src/Interpreters/InterpreterCreateQuery.h

@@ -74,7 +74,8 @@ public:
     /// Obtain information about columns, their types, default values and column comments,
     ///  for case when columns in CREATE query is specified explicitly.
     static ColumnsDescription getColumnsDescription(const ASTExpressionList & columns, ContextPtr context, LoadingStrictnessLevel mode, bool is_restore_from_backup = false);
-    static ConstraintsDescription getConstraintsDescription(const ASTExpressionList * constraints);
+    static ConstraintsDescription
+    getConstraintsDescription(const ASTExpressionList * constraints, const ColumnsDescription & columns, ContextPtr local_context);
 
     static void prepareOnClusterQuery(ASTCreateQuery & create, ContextPtr context, const String & cluster_name);
 

src/Interpreters/InterpreterSystemQuery.cpp

@@ -885,7 +885,7 @@ StoragePtr InterpreterSystemQuery::tryRestartReplica(const StorageID & replica,
     create.attach = true;
 
     auto columns = InterpreterCreateQuery::getColumnsDescription(*create.columns_list->columns, system_context, LoadingStrictnessLevel::ATTACH);
-    auto constraints = InterpreterCreateQuery::getConstraintsDescription(create.columns_list->constraints);
+    auto constraints = InterpreterCreateQuery::getConstraintsDescription(create.columns_list->constraints, columns, system_context);
     auto data_path = database->getTableDataPath(create);
 
     table = StorageFactory::instance().get(create,

src/Storages/IStorage.cpp

@@ -298,7 +298,7 @@ std::optional<CheckResult> IStorage::checkDataNext(DataValidationTasksPtr & /* c
     return {};
 }
 
-void IStorage::adjustCreateQueryForBackup(ASTPtr &) const
+void IStorage::adjustCreateQueryForBackup(ASTPtr &, ContextPtr) const
 {
 }
 

src/Storages/IStorage.h

@@ -245,7 +245,7 @@ public:
     bool isVirtualColumn(const String & column_name, const StorageMetadataPtr & metadata_snapshot) const;
 
     /// Modify a CREATE TABLE query to make a variant which must be written to a backup.
-    virtual void adjustCreateQueryForBackup(ASTPtr & create_query) const;
+    virtual void adjustCreateQueryForBackup(ASTPtr & create_query, ContextPtr context) const;
 
     /// Makes backup entries to backup the data of this storage.
     virtual void backupData(BackupEntriesCollector & backup_entries_collector, const String & data_path_in_backup, const std::optional<ASTs> & partitions);

src/Storages/StorageMergeTree.cpp

@@ -347,11 +347,13 @@ void StorageMergeTree::alter(
     if (commands.isSettingsAlter())
     {
         changeSettings(new_metadata.settings_changes, table_lock_holder);
+        /// It is safe to ignore exceptions here as only settings are changed, which is not validated in `alterTable`
         DatabaseCatalog::instance().getDatabase(table_id.database_name)->alterTable(local_context, table_id, new_metadata);
     }
     else if (commands.isCommentAlter())
     {
         setInMemoryMetadata(new_metadata);
+        /// It is safe to ignore exceptions here as only the comment changed, which is not validated in `alterTable`
         DatabaseCatalog::instance().getDatabase(table_id.database_name)->alterTable(local_context, table_id, new_metadata);
     }
     else
@@ -381,9 +383,18 @@ void StorageMergeTree::alter(
             checkTTLExpressions(new_metadata, old_metadata);
             /// Reinitialize primary key because primary key column types might have changed.
             setProperties(new_metadata, old_metadata, false, local_context);
-
-            DatabaseCatalog::instance().getDatabase(table_id.database_name)->alterTable(local_context, table_id, new_metadata);
-
+            try
+            {
+                DatabaseCatalog::instance().getDatabase(table_id.database_name)->alterTable(local_context, table_id, new_metadata);
+            }
+            catch (...)
+            {
+                LOG_ERROR(log, "Failed to alter table in database, reverting changes");
+                changeSettings(old_metadata.settings_changes, table_lock_holder);
+                checkTTLExpressions(old_metadata, new_metadata);
+                setProperties(old_metadata, new_metadata, false, local_context);
+                throw;
+            }
             if (!maybe_mutation_commands.empty())
                 mutation_version = startMutation(maybe_mutation_commands, local_context);
         }

src/Storages/StorageReplicatedMergeTree.cpp

@@ -1493,7 +1493,16 @@ void StorageReplicatedMergeTree::setTableStructure(const StorageID & table_id, c
     checkTTLExpressions(new_metadata, old_metadata);
     setProperties(new_metadata, old_metadata);
 
-    DatabaseCatalog::instance().getDatabase(table_id.database_name)->alterTable(local_context, table_id, new_metadata);
+    try
+    {
+        DatabaseCatalog::instance().getDatabase(table_id.database_name)->alterTable(local_context, table_id, new_metadata);
+    }
+    catch (...)
+    {
+        LOG_ERROR(log, "Failed to set table structure, reverting changes");
+        setProperties(old_metadata, new_metadata);
+        throw;
+    }
 }
 
 
@@ -6077,40 +6086,42 @@ void StorageReplicatedMergeTree::alter(
     auto table_id = getStorageID();
     const auto & query_settings = query_context->getSettingsRef();
 
+    StorageInMemoryMetadata future_metadata = getInMemoryMetadata();
+    commands.apply(future_metadata, query_context);
+
     if (commands.isSettingsAlter())
     {
         /// We don't replicate storage_settings_ptr ALTER. It's local operation.
         /// Also we don't upgrade alter lock to table structure lock.
-        StorageInMemoryMetadata future_metadata = getInMemoryMetadata();
-        commands.apply(future_metadata, query_context);
-
         merge_strategy_picker.refreshState();
 
         changeSettings(future_metadata.settings_changes, table_lock_holder);
 
+        /// It is safe to ignore exceptions here as only settings are changed, which is not validated in `alterTable`
         DatabaseCatalog::instance().getDatabase(table_id.database_name)->alterTable(query_context, table_id, future_metadata);
         return;
     }
 
     if (commands.isCommentAlter())
     {
-        StorageInMemoryMetadata future_metadata = getInMemoryMetadata();
-        commands.apply(future_metadata, query_context);
-
         setInMemoryMetadata(future_metadata);
 
+        /// It is safe to ignore exceptions here as only the comment is changed, which is not validated in `alterTable`
         DatabaseCatalog::instance().getDatabase(table_id.database_name)->alterTable(query_context, table_id, future_metadata);
         return;
     }
 
     if (!query_settings.allow_suspicious_primary_key)
     {
-        StorageInMemoryMetadata future_metadata = getInMemoryMetadata();
-        commands.apply(future_metadata, query_context);
-
         MergeTreeData::verifySortingKey(future_metadata.sorting_key);
     }
 
+    {
+        /// Call applyMetadataChangesToCreateQuery to validate the resulting CREATE query
+        auto ast = DatabaseCatalog::instance().getDatabase(table_id.database_name)->getCreateTableQuery(table_id.table_name, query_context);
+        applyMetadataChangesToCreateQuery(ast, future_metadata, query_context);
+    }
+
     auto ast_to_str = [](ASTPtr query) -> String
     {
         if (!query)
@@ -6156,9 +6167,6 @@ void StorageReplicatedMergeTree::alter(
 
         auto current_metadata = getInMemoryMetadataPtr();
 
-        StorageInMemoryMetadata future_metadata = *current_metadata;
-        commands.apply(future_metadata, query_context);
-
         ReplicatedMergeTreeTableMetadata future_metadata_in_zk(*this, current_metadata);
         if (ast_to_str(future_metadata.sorting_key.definition_ast) != ast_to_str(current_metadata->sorting_key.definition_ast))
         {
@@ -6226,6 +6234,8 @@ void StorageReplicatedMergeTree::alter(
                 setInMemoryMetadata(metadata_copy);
             }
 
+            /// Only the comment and/or settings changed here, so it is okay to assume alterTable won't throw as neither
+            /// of them are validated in alterTable.
             DatabaseCatalog::instance().getDatabase(table_id.database_name)->alterTable(query_context, table_id, metadata_copy);
         }
 
@@ -6297,7 +6307,7 @@ void StorageReplicatedMergeTree::alter(
             /// so we have to update metadata of DatabaseReplicated here.
             String metadata_zk_path = fs::path(txn->getDatabaseZooKeeperPath()) / "metadata" / escapeForFileName(table_id.table_name);
             auto ast = DatabaseCatalog::instance().getDatabase(table_id.database_name)->getCreateTableQuery(table_id.table_name, query_context);
-            applyMetadataChangesToCreateQuery(ast, future_metadata);
+            applyMetadataChangesToCreateQuery(ast, future_metadata, query_context);
             ops.emplace_back(zkutil::makeSetRequest(metadata_zk_path, getObjectDefinitionFromCreateQuery(ast), -1));
         }
 
@@ -10451,7 +10461,7 @@ void StorageReplicatedMergeTree::createAndStoreFreezeMetadata(DiskPtr disk, Data
 }
 
 
-void StorageReplicatedMergeTree::adjustCreateQueryForBackup(ASTPtr & create_query) const
+void StorageReplicatedMergeTree::adjustCreateQueryForBackup(ASTPtr & create_query, ContextPtr local_context) const
 {
     try
     {
@@ -10462,8 +10472,8 @@ void StorageReplicatedMergeTree::adjustCreateQueryForBackup(ASTPtr & create_quer
 
         auto current_metadata = getInMemoryMetadataPtr();
         auto metadata_diff = ReplicatedMergeTreeTableMetadata(*this, current_metadata).checkAndFindDiff(metadata_from_entry, current_metadata->getColumns(), getContext());
-        auto adjusted_metadata = metadata_diff.getNewMetadata(columns_from_entry, getContext(), *current_metadata);
-        applyMetadataChangesToCreateQuery(create_query, adjusted_metadata);
+        auto adjusted_metadata = metadata_diff.getNewMetadata(columns_from_entry, local_context, *current_metadata);
+        applyMetadataChangesToCreateQuery(create_query, adjusted_metadata, local_context);
     }
     catch (...)
     {

src/Storages/StorageReplicatedMergeTree.h

@@ -232,7 +232,7 @@ public:
     bool canUseAdaptiveGranularity() const override;
 
     /// Modify a CREATE TABLE query to make a variant which must be written to a backup.
-    void adjustCreateQueryForBackup(ASTPtr & create_query) const override;
+    void adjustCreateQueryForBackup(ASTPtr & create_query, ContextPtr local_context) const override;
 
     /// Makes backup entries to backup the data of the storage.
     void backupData(BackupEntriesCollector & backup_entries_collector, const String & data_path_in_backup, const std::optional<ASTs> & partitions) override;

tests/queries/0_stateless/03224_invalid_alter.reference

@@ -0,0 +1,8 @@
+test	testa	testa	testc
+test2	test2a	test2a	test2c
+localhost	9000	0		0	0
+localhost	9000	0		0	0
+localhost	9000	0		0	0
+localhost	9000	0		0	0
+test3	test3a	test3a	test3c
+test4	test4a	test4a	test4c

tests/queries/0_stateless/03224_invalid_alter.sql

@@ -0,0 +1,72 @@
+CREATE TABLE test
+(
+    str String,
+    column_with_alias String MATERIALIZED concat(str, 'a' AS a),
+)
+ENGINE = MergeTree()
+ORDER BY tuple();
+
+-- The columns are named the same intentionally. It can catch an issue with
+ALTER TABLE test ADD COLUMN invalid_column String MATERIALIZED concat(str, 'b' AS a); -- { serverError MULTIPLE_EXPRESSIONS_FOR_ALIAS }
+ALTER TABLE test ADD COLUMN invalid_column String DEFAULT concat(str, 'b' AS a); -- { serverError MULTIPLE_EXPRESSIONS_FOR_ALIAS }
+-- alias is defined exactly the same
+ALTER TABLE test ADD COLUMN valid_column_1 String DEFAULT concat(str, 'a' AS a);
+-- different alias
+ALTER TABLE test ADD COLUMN valid_column_2 String MATERIALIZED concat(str, 'c' AS c);
+-- do one insert to make sure we can insert into the table
+INSERT INTO test(str) VALUES ('test');
+SELECT str, column_with_alias, valid_column_1, valid_column_2 FROM test;
+DROP TABLE test;
+
+CREATE TABLE test2
+(
+    str String,
+    column_with_alias String MATERIALIZED concat(str, 'a' AS a),
+)
+ENGINE = ReplicatedMergeTree('/clickhouse/03224_invalid_alter/{database}/{table}', 'r1')
+ORDER BY tuple();
+
+ALTER TABLE test2 ADD COLUMN invalid_column String MATERIALIZED concat(str, 'b' AS a); -- { serverError MULTIPLE_EXPRESSIONS_FOR_ALIAS }
+ALTER TABLE test2 ADD COLUMN invalid_column String DEFAULT concat(str, 'b' AS a); -- { serverError MULTIPLE_EXPRESSIONS_FOR_ALIAS }
+ALTER TABLE test2 ADD COLUMN valid_column_1 String DEFAULT concat(str, 'a' AS a);
+ALTER TABLE test2 ADD COLUMN valid_column_2 String MATERIALIZED concat(str, 'c' AS c);
+INSERT INTO test2(str) VALUES ('test2');
+SELECT str, column_with_alias, valid_column_1, valid_column_2 FROM test2;
+
+DROP DATABASE {CLICKHOUSE_DATABASE:Identifier};
+
+CREATE DATABASE {CLICKHOUSE_DATABASE:Identifier} ON CLUSTER test_shard_localhost ENGINE = Atomic;
+
+CREATE TABLE test3 ON CLUSTER test_shard_localhost
+(
+    str String,
+    column_with_alias String MATERIALIZED concat(str, 'a' AS a),
+)
+ENGINE = ReplicatedMergeTree('/clickhouse/03224_invalid_alter/{database}_atomic/{table}', 'r1')
+ORDER BY tuple();
+
+ALTER TABLE test3 ON CLUSTER test_shard_localhost ADD COLUMN invalid_column String MATERIALIZED concat(str, 'b' AS a) FORMAT Null SETTINGS distributed_ddl_output_mode='throw'; -- { serverError MULTIPLE_EXPRESSIONS_FOR_ALIAS }
+ALTER TABLE test3 ON CLUSTER test_shard_localhost ADD COLUMN invalid_column String DEFAULT concat(str, 'b' AS a) FORMAT Null SETTINGS distributed_ddl_output_mode='throw'; -- { serverError MULTIPLE_EXPRESSIONS_FOR_ALIAS }
+ALTER TABLE test3 ON CLUSTER test_shard_localhost ADD COLUMN valid_column_1 String DEFAULT concat(str, 'a' AS a);
+ALTER TABLE test3 ON CLUSTER test_shard_localhost ADD COLUMN valid_column_2 String MATERIALIZED concat(str, 'c' AS c);
+INSERT INTO test3(str) VALUES ('test3');
+SELECT str, column_with_alias, valid_column_1, valid_column_2 FROM test3;
+
+DROP DATABASE {CLICKHOUSE_DATABASE:Identifier};
+CREATE DATABASE {CLICKHOUSE_DATABASE:Identifier} ENGINE = Replicated('/clickhouse/03224_invalid_alter/{database}_replicated', 'shard1', 'replica1') FORMAT Null;
+
+CREATE TABLE test4
+(
+    str String,
+    column_with_alias String MATERIALIZED concat(str, 'a' AS a),
+)
+ENGINE = ReplicatedMergeTree()
+ORDER BY tuple()
+FORMAT Null;
+
+ALTER TABLE test4 ADD COLUMN invalid_column String MATERIALIZED concat(str, 'b' AS a) FORMAT Null SETTINGS distributed_ddl_output_mode='throw'; -- { serverError MULTIPLE_EXPRESSIONS_FOR_ALIAS }
+ALTER TABLE test4 ADD COLUMN invalid_column String DEFAULT concat(str, 'b' AS a) FORMAT Null SETTINGS distributed_ddl_output_mode='throw'; -- { serverError MULTIPLE_EXPRESSIONS_FOR_ALIAS }
+ALTER TABLE test4 ADD COLUMN valid_column_1 String DEFAULT concat(str, 'a' AS a) FORMAT Null SETTINGS distributed_ddl_output_mode='throw';
+ALTER TABLE test4 ADD COLUMN valid_column_2 String MATERIALIZED concat(str, 'c' AS c) FORMAT Null SETTINGS distributed_ddl_output_mode='throw';
+INSERT INTO test4(str) VALUES ('test4');
+SELECT str, column_with_alias, valid_column_1, valid_column_2 FROM test4;