From 5c6c318737ab9fb8fbcc2f853a76f0c69ea9d173 Mon Sep 17 00:00:00 2001 From: alesapin Date: Thu, 25 Feb 2021 13:07:48 +0300 Subject: [PATCH] Restrict mutations for engines which doesn't support them --- src/Interpreters/InterpreterAlterQuery.cpp | 1 + src/Storages/IStorage.cpp | 5 ++++ src/Storages/IStorage.h | 5 ++++ src/Storages/MergeTree/MergeTreeData.cpp | 6 ++++ src/Storages/MergeTree/MergeTreeData.h | 4 +++ src/Storages/StorageMaterializedView.cpp | 6 ++++ src/Storages/StorageMaterializedView.h | 2 ++ src/Storages/StorageMemory.cpp | 5 ++++ src/Storages/StorageMemory.h | 1 + .../01745_alter_delete_view.reference | 4 +++ .../0_stateless/01745_alter_delete_view.sql | 28 +++++++++++++++++++ 11 files changed, 67 insertions(+) create mode 100644 tests/queries/0_stateless/01745_alter_delete_view.reference create mode 100644 tests/queries/0_stateless/01745_alter_delete_view.sql diff --git a/src/Interpreters/InterpreterAlterQuery.cpp b/src/Interpreters/InterpreterAlterQuery.cpp index bf624507574..6294b31cc8c 100644 --- a/src/Interpreters/InterpreterAlterQuery.cpp +++ b/src/Interpreters/InterpreterAlterQuery.cpp @@ -104,6 +104,7 @@ BlockIO InterpreterAlterQuery::execute() if (!mutation_commands.empty()) { + table->checkMutationIsPossible(mutation_commands, context.getSettingsRef()); MutationsInterpreter(table, metadata_snapshot, mutation_commands, context, false).validate(); table->mutate(mutation_commands, context); } diff --git a/src/Storages/IStorage.cpp b/src/Storages/IStorage.cpp index 5f500518516..2400b0587ba 100644 --- a/src/Storages/IStorage.cpp +++ b/src/Storages/IStorage.cpp @@ -145,6 +145,11 @@ void IStorage::checkAlterIsPossible(const AlterCommands & commands, const Settin } } +void IStorage::checkMutationIsPossible(const MutationCommands & /*commands*/, const Settings & /*settings*/) const +{ + throw Exception("Table engine " + getName() + " doesn't support mutations", ErrorCodes::NOT_IMPLEMENTED); +} + void IStorage::checkAlterPartitionIsPossible( const PartitionCommands & /*commands*/, const StorageMetadataPtr & /*metadata_snapshot*/, const Settings & /*settings*/) const { diff --git a/src/Storages/IStorage.h b/src/Storages/IStorage.h index 651688f41bb..1a27dbd637f 100644 --- a/src/Storages/IStorage.h +++ b/src/Storages/IStorage.h @@ -364,6 +364,11 @@ public: */ virtual void checkAlterIsPossible(const AlterCommands & commands, const Settings & settings) const; + /** + * Checks that mutation commands can be applied to storage. + */ + virtual void checkMutationIsPossible(const MutationCommands & commands, const Settings & settings) const; + /** ALTER tables with regard to its partitions. * Should handle locks for each command on its own. */ diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index b09f068f509..2d841b98c59 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -1670,6 +1670,12 @@ void MergeTreeData::checkAlterIsPossible(const AlterCommands & commands, const S } } + +void MergeTreeData::checkMutationIsPossible(const MutationCommands & /*commands*/, const Settings & /*settings*/) const +{ + /// Some validation will be added +} + MergeTreeDataPartType MergeTreeData::choosePartType(size_t bytes_uncompressed, size_t rows_count) const { const auto settings = getSettings(); diff --git a/src/Storages/MergeTree/MergeTreeData.h b/src/Storages/MergeTree/MergeTreeData.h index 2aefa66ac58..09cf017d220 100644 --- a/src/Storages/MergeTree/MergeTreeData.h +++ b/src/Storages/MergeTree/MergeTreeData.h @@ -519,6 +519,10 @@ public: /// If something is wrong, throws an exception. void checkAlterIsPossible(const AlterCommands & commands, const Settings & settings) const override; + /// Checks if the Mutation can be performed. + /// (currenly no additional checks: always ok) + void checkMutationIsPossible(const MutationCommands & commands, const Settings & settings) const override; + /// Checks that partition name in all commands is valid void checkAlterPartitionIsPossible(const PartitionCommands & commands, const StorageMetadataPtr & metadata_snapshot, const Settings & settings) const override; diff --git a/src/Storages/StorageMaterializedView.cpp b/src/Storages/StorageMaterializedView.cpp index 325bf3d2f74..2d211c8061b 100644 --- a/src/Storages/StorageMaterializedView.cpp +++ b/src/Storages/StorageMaterializedView.cpp @@ -320,6 +320,12 @@ void StorageMaterializedView::checkAlterIsPossible(const AlterCommands & command } } +void StorageMaterializedView::checkMutationIsPossible(const MutationCommands & commands, const Settings & settings) const +{ + checkStatementCanBeForwarded(); + getTargetTable()->checkMutationIsPossible(commands, settings); +} + Pipe StorageMaterializedView::alterPartition( const StorageMetadataPtr & metadata_snapshot, const PartitionCommands & commands, const Context & context) { diff --git a/src/Storages/StorageMaterializedView.h b/src/Storages/StorageMaterializedView.h index 94e4295cd34..4b10cf7a9b5 100644 --- a/src/Storages/StorageMaterializedView.h +++ b/src/Storages/StorageMaterializedView.h @@ -52,6 +52,8 @@ public: void alter(const AlterCommands & params, const Context & context, TableLockHolder & table_lock_holder) override; + void checkMutationIsPossible(const MutationCommands & commands, const Settings & settings) const override; + void checkAlterIsPossible(const AlterCommands & commands, const Settings & settings) const override; Pipe alterPartition(const StorageMetadataPtr & metadata_snapshot, const PartitionCommands & commands, const Context & context) override; diff --git a/src/Storages/StorageMemory.cpp b/src/Storages/StorageMemory.cpp index d7b0ae055ab..d98cd4212e9 100644 --- a/src/Storages/StorageMemory.cpp +++ b/src/Storages/StorageMemory.cpp @@ -253,6 +253,11 @@ static inline void updateBlockData(Block & old_block, const Block & new_block) } } +void StorageMemory::checkMutationIsPossible(const MutationCommands & /*commands*/, const Settings & /*settings*/) const +{ + /// Some validation will be added +} + void StorageMemory::mutate(const MutationCommands & commands, const Context & context) { std::lock_guard lock(mutex); diff --git a/src/Storages/StorageMemory.h b/src/Storages/StorageMemory.h index db71c13ca99..b7fa4d7b222 100644 --- a/src/Storages/StorageMemory.h +++ b/src/Storages/StorageMemory.h @@ -51,6 +51,7 @@ public: void drop() override; + void checkMutationIsPossible(const MutationCommands & commands, const Settings & settings) const override; void mutate(const MutationCommands & commands, const Context & context) override; void truncate(const ASTPtr &, const StorageMetadataPtr &, const Context &, TableExclusiveLockHolder &) override; diff --git a/tests/queries/0_stateless/01745_alter_delete_view.reference b/tests/queries/0_stateless/01745_alter_delete_view.reference new file mode 100644 index 00000000000..dc3ab50ab0d --- /dev/null +++ b/tests/queries/0_stateless/01745_alter_delete_view.reference @@ -0,0 +1,4 @@ +1 1 +2 1 +1 1 +2 1 diff --git a/tests/queries/0_stateless/01745_alter_delete_view.sql b/tests/queries/0_stateless/01745_alter_delete_view.sql new file mode 100644 index 00000000000..c242f1be63e --- /dev/null +++ b/tests/queries/0_stateless/01745_alter_delete_view.sql @@ -0,0 +1,28 @@ +DROP VIEW IF EXISTS test_view; +DROP TABLE IF EXISTS test_table; + +CREATE TABLE test_table +( + f1 Int32, + f2 Int32, + pk Int32 +) +ENGINE = MergeTree() +ORDER BY f1 +PARTITION BY pk; + +CREATE VIEW test_view AS +SELECT f1, f2 +FROM test_table +WHERE pk = 2; + +INSERT INTO test_table (f1, f2, pk) VALUES (1,1,1), (1,1,2), (2,1,1), (2,1,2); + +SELECT * FROM test_view ORDER BY f1, f2; + +ALTER TABLE test_view DELETE WHERE pk = 2; --{serverError 48} + +SELECT * FROM test_view ORDER BY f1, f2; + +DROP VIEW IF EXISTS test_view; +DROP TABLE IF EXISTS test_table;