diff --git a/dbms/src/DataStreams/PushingToViewsBlockOutputStream.cpp b/dbms/src/DataStreams/PushingToViewsBlockOutputStream.cpp index 90e2d95ad15..260ecf6bdb9 100644 --- a/dbms/src/DataStreams/PushingToViewsBlockOutputStream.cpp +++ b/dbms/src/DataStreams/PushingToViewsBlockOutputStream.cpp @@ -8,7 +8,6 @@ #include #include #include -#include #include #include #include @@ -51,8 +50,10 @@ PushingToViewsBlockOutputStream::PushingToViewsBlockOutputStream( ASTPtr query; BlockOutputStreamPtr out; - if (auto * materialized_view = dynamic_cast(dependent_table.get())) + if (auto * materialized_view = dynamic_cast(dependent_table.get())) { + addTableLock(materialized_view->lockStructureForShare(true, context.getInitialQueryId())); + StoragePtr inner_table = materialized_view->getTargetTable(); auto inner_table_id = inner_table->getStorageID(); query = materialized_view->getInnerQuery(); diff --git a/dbms/src/Databases/DatabaseOrdinary.cpp b/dbms/src/Databases/DatabaseOrdinary.cpp index a8bb8128b6a..2188b7b5e80 100644 --- a/dbms/src/Databases/DatabaseOrdinary.cpp +++ b/dbms/src/Databases/DatabaseOrdinary.cpp @@ -252,20 +252,23 @@ void DatabaseOrdinary::alterTable( ast->replace(ast_create_query.select, metadata.select); } - ASTStorage & storage_ast = *ast_create_query.storage; - /// ORDER BY may change, but cannot appear, it's required construction - if (metadata.order_by_ast && storage_ast.order_by) - storage_ast.set(storage_ast.order_by, metadata.order_by_ast); + /// MaterializedView is one type of CREATE query without storage. + if (ast_create_query.storage) + { + ASTStorage & storage_ast = *ast_create_query.storage; + /// ORDER BY may change, but cannot appear, it's required construction + if (metadata.order_by_ast && storage_ast.order_by) + storage_ast.set(storage_ast.order_by, metadata.order_by_ast); - if (metadata.primary_key_ast) - storage_ast.set(storage_ast.primary_key, metadata.primary_key_ast); + if (metadata.primary_key_ast) + storage_ast.set(storage_ast.primary_key, metadata.primary_key_ast); - if (metadata.ttl_for_table_ast) - storage_ast.set(storage_ast.ttl_table, metadata.ttl_for_table_ast); - - if (metadata.settings_ast) - storage_ast.set(storage_ast.settings, metadata.settings_ast); + if (metadata.ttl_for_table_ast) + storage_ast.set(storage_ast.ttl_table, metadata.ttl_for_table_ast); + if (metadata.settings_ast) + storage_ast.set(storage_ast.settings, metadata.settings_ast); + } statement = getObjectDefinitionFromCreateQuery(ast); { diff --git a/dbms/src/Storages/StorageMaterializedView.cpp b/dbms/src/Storages/StorageMaterializedView.cpp index 814036b88a5..988168256e7 100644 --- a/dbms/src/Storages/StorageMaterializedView.cpp +++ b/dbms/src/Storages/StorageMaterializedView.cpp @@ -37,7 +37,7 @@ static inline String generateInnerTableName(const String & table_name) return ".inner." + table_name; } -static StorageID extractDependentTableFromSelectQuery(ASTSelectQuery & query, Context & context, bool add_default_db = true) +static StorageID extractDependentTableFromSelectQuery(ASTSelectQuery & query, const Context & context, bool add_default_db = true) { if (add_default_db) { @@ -117,22 +117,23 @@ StorageMaterializedView::StorageMaterializedView( inner_query = query.select->list_of_selects->children.at(0); auto & select_query = inner_query->as(); - select_table_id = extractDependentTableFromSelectQuery(select_query, local_context); checkAllowedQueries(select_query); + select_table_id = extractDependentTableFromSelectQuery(select_query, local_context); + if (!has_inner_table) target_table_id = StorageID(query.to_database, query.to_table); else if (attach_) { /// If there is an ATTACH request, then the internal table must already be created. - target_table_id = StorageID(table_id_.database_name, generateInnerTableName(table_id_.table_name)); + target_table_id = StorageID(getStorageID().database_name, generateInnerTableName(getStorageID().table_name)); } else { /// We will create a query to create an internal table. auto manual_create_query = std::make_shared(); - manual_create_query->database = table_id_.database_name; - manual_create_query->table = generateInnerTableName(table_id_.table_name); + manual_create_query->database = getStorageID().database_name; + manual_create_query->table = generateInnerTableName(getStorageID().table_name); auto new_columns_list = std::make_shared(); new_columns_list->set(new_columns_list->columns, query.columns_list->columns->ptr()); @@ -148,7 +149,7 @@ StorageMaterializedView::StorageMaterializedView( } if (!select_table_id.empty()) - global_context.addDependency(select_table_id, table_id_); + global_context.addDependency(select_table_id, getStorageID()); } NameAndTypePair StorageMaterializedView::getColumn(const String & column_name) const @@ -261,6 +262,37 @@ void StorageMaterializedView::alter( auto table_id = getStorageID(); StorageInMemoryMetadata metadata = getInMemoryMetadata(); params.apply(metadata); + + /// start modify query + if (context.getSettingsRef().allow_experimental_alter_materialized_view_structure) + { + auto & new_select = metadata.select->as(); + + if (new_select.list_of_selects->children.size() != 1) + throw Exception("UNION is not supported for MATERIALIZED VIEW", ErrorCodes::QUERY_IS_NOT_SUPPORTED_IN_MATERIALIZED_VIEW); + + auto & new_inner_query = new_select.list_of_selects->children.at(0); + auto & select_query = new_inner_query->as(); + checkAllowedQueries(select_query); + + auto new_select_table_id = extractDependentTableFromSelectQuery(select_query, context); + + { + auto context_lock = global_context.getLock(); + + if (!select_table_id.empty()) + global_context.removeDependency(select_table_id, getStorageID()); + + if (!new_select_table_id.empty()) + global_context.addDependency(new_select_table_id, getStorageID()); + + select_table_id = new_select_table_id; + select = metadata.select; + inner_query = new_inner_query; + } + } + /// end modify query + context.getDatabase(table_id.database_name)->alterTable(context, table_id.table_name, metadata); setColumns(std::move(metadata.columns)); } @@ -270,7 +302,13 @@ void StorageMaterializedView::checkAlterIsPossible(const AlterCommands & command { if (settings.allow_experimental_alter_materialized_view_structure) { - throw Exception("work in progress", ErrorCodes::NOT_IMPLEMENTED); + for (const auto & command : commands) + { + if (!command.isCommentAlter() && command.type != AlterCommand::MODIFY_QUERY) + throw Exception( + "Alter of type '" + alterTypeToString(command.type) + "' is not supported by storage " + getName(), + ErrorCodes::NOT_IMPLEMENTED); + } } else { diff --git a/dbms/tests/queries/0_stateless/01019_alter_materialized_view_atomic.reference b/dbms/tests/queries/0_stateless/01019_alter_materialized_view_atomic.reference new file mode 100755 index 00000000000..29d6383b52c --- /dev/null +++ b/dbms/tests/queries/0_stateless/01019_alter_materialized_view_atomic.reference @@ -0,0 +1 @@ +100 diff --git a/dbms/tests/queries/0_stateless/01019_alter_materialized_view_atomic.sh b/dbms/tests/queries/0_stateless/01019_alter_materialized_view_atomic.sh new file mode 100755 index 00000000000..e36c91ff513 --- /dev/null +++ b/dbms/tests/queries/0_stateless/01019_alter_materialized_view_atomic.sh @@ -0,0 +1,60 @@ +#!/usr/bin/env bash + +set -e + +CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +. $CURDIR/../shell_config.sh + +$CLICKHOUSE_CLIENT --multiquery </dev/null && break + done + ) +done + +# Enough alters. +kill -INT $alter_pid + +wait + +# This was a fun ride. +$CLICKHOUSE_CLIENT -q "SELECT count() FROM mv;" diff --git a/dbms/tests/queries/0_stateless/01019_alter_materialized_view_consistent.reference b/dbms/tests/queries/0_stateless/01019_alter_materialized_view_consistent.reference new file mode 100755 index 00000000000..726246fba3f --- /dev/null +++ b/dbms/tests/queries/0_stateless/01019_alter_materialized_view_consistent.reference @@ -0,0 +1 @@ +inconsistencies 0 diff --git a/dbms/tests/queries/0_stateless/01019_alter_materialized_view_consistent.sh b/dbms/tests/queries/0_stateless/01019_alter_materialized_view_consistent.sh new file mode 100755 index 00000000000..51ff163b730 --- /dev/null +++ b/dbms/tests/queries/0_stateless/01019_alter_materialized_view_consistent.sh @@ -0,0 +1,75 @@ +#!/usr/bin/env bash + +set -e + +CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +. $CURDIR/../shell_config.sh + +$CLICKHOUSE_CLIENT --multiquery </dev/null & + done + + wait + done +} + +function alter_thread() { + trap 'exit' INT + + ALTER[0]="ALTER TABLE mv MODIFY QUERY SELECT v == 1 as test, v as case FROM src_a;" + ALTER[1]="ALTER TABLE mv MODIFY QUERY SELECT v == 2 as test, v as case FROM src_b;" + + while true; do + $CLICKHOUSE_CLIENT --allow_experimental_alter_materialized_view_structure=1 \ + -q "${ALTER[$RANDOM % 2]}" + sleep "0.0$RANDOM" + done +} + +insert_thread & +insert_thread_pid=$! + +alter_thread & +alter_thread_pid=$! + +while true; do + is_done=$($CLICKHOUSE_CLIENT -q "SELECT countIf(case = 1) > 0 AND countIf(case = 2) > 0 FROM mv;") + + if [ "$is_done" -eq "1" ]; then + break + fi +done + +kill -INT $insert_thread_pid +kill -INT $alter_thread_pid + +wait + +$CLICKHOUSE_CLIENT -q "SELECT 'inconsistencies', count() FROM mv WHERE test == 0;" diff --git a/dbms/tests/queries/0_stateless/01019_alter_materialized_view_query.reference b/dbms/tests/queries/0_stateless/01019_alter_materialized_view_query.reference index ce6ee26c6ed..efe5740033c 100644 --- a/dbms/tests/queries/0_stateless/01019_alter_materialized_view_query.reference +++ b/dbms/tests/queries/0_stateless/01019_alter_materialized_view_query.reference @@ -1,13 +1,13 @@ 1 -1 2 2 3 -3 -1 0 +4 +6 1 0 2 0 2 0 3 0 -3 0 -42 0 +4 0 +6 0 +84 1 diff --git a/dbms/tests/queries/0_stateless/01019_alter_materialized_view_query.sql b/dbms/tests/queries/0_stateless/01019_alter_materialized_view_query.sql index 6768a8265ca..d6a40671dc4 100644 --- a/dbms/tests/queries/0_stateless/01019_alter_materialized_view_query.sql +++ b/dbms/tests/queries/0_stateless/01019_alter_materialized_view_query.sql @@ -1,5 +1,3 @@ --- Just testing syntax for now. - DROP TABLE IF EXISTS src; DROP TABLE IF EXISTS dest; DROP TABLE IF EXISTS pipe; @@ -15,13 +13,12 @@ INSERT INTO src VALUES (1), (2), (3); SET allow_experimental_alter_materialized_view_structure = 1; -- Live alter which changes query logic and adds an extra column. --- This is not implemented yet and this test is just a draft. ALTER TABLE pipe MODIFY QUERY SELECT v * 2 as v, 1 as v2 - FROM src; -- { serverError 48 } + FROM src; INSERT INTO src VALUES (1), (2), (3);