Merge pull request ClickHouse#80616 from ClickHouse/revert-78739-delete_to_truncate

Revert "Replace DELETE FROM ... WHERE 1 queries to TRUNCATE"

Author: Algunenano

src/Interpreters/InterpreterAlterQuery.cpp

@@ -4,11 +4,6 @@
 
 #include <Access/Common/AccessRightsElement.h>
 #include <Common/typeid_cast.h>
-#include <Storages/TableLockHolder.h>
-#include <Parsers/ASTLiteral.h>
-#include <Parsers/CommonParsers.h>
-#include <Parsers/ParserDropQuery.h>
-#include <Parsers/parseQuery.h>
 #include <Core/Settings.h>
 #include <Core/ServerSettings.h>
 #include <Databases/DatabaseFactory.h>
@@ -142,6 +137,7 @@ BlockIO InterpreterAlterQuery::executeToTable(const ASTAlterQuery & alter)
     checkStorageSupportsTransactionsIfNeeded(table, getContext());
     if (table->isStaticStorage())
         throw Exception(ErrorCodes::TABLE_IS_READ_ONLY, "Table is read-only");
+    auto table_lock = table->lockForShare(getContext()->getCurrentQueryId(), getContext()->getSettingsRef()[Setting::lock_acquire_timeout]);
 
     if (modify_query)
     {
@@ -157,9 +153,6 @@ BlockIO InterpreterAlterQuery::executeToTable(const ASTAlterQuery & alter)
     AlterCommands alter_commands;
     PartitionCommands partition_commands;
     MutationCommands mutation_commands;
-    bool is_truncate = false;
-    bool forbid_truncate = false;
-
     for (const auto & child : alter.command_list->children)
     {
         auto * command_ast = child->as<ASTAlterCommand>();
@@ -173,15 +166,8 @@ BlockIO InterpreterAlterQuery::executeToTable(const ASTAlterQuery & alter)
         }
         else if (auto mut_command = MutationCommand::parse(command_ast))
         {
-            /// ALTER TABLE ... DELETE WHERE 1 should be executed as truncate
-            if (mut_command->type == MutationCommand::DELETE && isAlwaysTruePredicate(mut_command->predicate))
-            {
-                is_truncate = true;
-            }
-            else if (mut_command->type == MutationCommand::UPDATE || mut_command->type == MutationCommand::DELETE)
+            if (mut_command->type == MutationCommand::UPDATE || mut_command->type == MutationCommand::DELETE)
             {
-                forbid_truncate = true;
-
                 /// TODO: add a check for result query size.
                 auto rewritten_command_ast = replaceNonDeterministicToScalars(*command_ast, getContext());
                 if (rewritten_command_ast)
@@ -206,45 +192,6 @@ BlockIO InterpreterAlterQuery::executeToTable(const ASTAlterQuery & alter)
             throw Exception(ErrorCodes::INCORRECT_QUERY, "Alter table with statistics is now disabled. Turn on allow_experimental_statistics");
     }
 
-    if (mutation_commands.hasNonEmptyMutationCommands() || !partition_commands.empty())
-    {
-        if (getContext()->getServerSettings()[ServerSetting::disable_insertion_and_mutation])
-            throw Exception(ErrorCodes::QUERY_IS_PROHIBITED, "Mutations are prohibited");
-    }
-
-    if (is_truncate && !forbid_truncate)
-    {
-        auto context = getContext();
-        context->checkAccess(AccessType::TRUNCATE, table_id);
-        table->checkTableCanBeDropped(context);
-
-        auto metadata_snapshot = table->getInMemoryMetadataPtr();
-
-        TableExclusiveLockHolder table_excl_lock;
-        /// We don't need any lock for ReplicatedMergeTree and for simple MergeTree
-        /// For the rest of tables types exclusive lock is needed
-        if (!std::dynamic_pointer_cast<MergeTreeData>(table))
-            table_excl_lock = table->lockExclusively(context->getCurrentQueryId(), context->getSettingsRef()[Setting::lock_acquire_timeout]);
-        String truncate_query = "TRUNCATE TABLE " + table->getStorageID().getFullTableName()
-            + (alter.cluster.empty() ? "" : " ON CLUSTER " + backQuoteIfNeed(alter.cluster));
-
-        ParserDropQuery parser;
-        auto current_query_ptr = parseQuery(
-            parser,
-            truncate_query.data(),
-            truncate_query.data() + truncate_query.size(),
-            "ALTER query",
-            0,
-            DBMS_DEFAULT_MAX_PARSER_DEPTH,
-            DBMS_DEFAULT_MAX_PARSER_BACKTRACKS);
-
-        /// Drop table data, don't touch metadata
-        table->truncate(current_query_ptr, metadata_snapshot, context, table_excl_lock);
-        return {};
-    }
-
-    auto table_lock = table->lockForShare(getContext()->getCurrentQueryId(), getContext()->getSettingsRef()[Setting::lock_acquire_timeout]);
-
     if (typeid_cast<DatabaseReplicated *>(database.get()))
     {
         int command_types_count = !mutation_commands.empty() + !partition_commands.empty() + !alter_commands.empty();
@@ -254,6 +201,12 @@ BlockIO InterpreterAlterQuery::executeToTable(const ASTAlterQuery & alter)
                                                          "to execute ALTERs of different types (replicated and non replicated) in single query");
     }
 
+    if (mutation_commands.hasNonEmptyMutationCommands() || !partition_commands.empty())
+    {
+        if (getContext()->getServerSettings()[ServerSetting::disable_insertion_and_mutation])
+            throw Exception(ErrorCodes::QUERY_IS_PROHIBITED, "Mutations are prohibited");
+    }
+
     if (!alter_commands.empty())
     {
         auto alter_lock = table->lockForAlter(getContext()->getSettingsRef()[Setting::lock_acquire_timeout]);

src/Interpreters/InterpreterDeleteQuery.cpp

@@ -14,9 +14,6 @@
 #include <Parsers/parseQuery.h>
 #include <Parsers/ParserAlterQuery.h>
 #include <Parsers/ASTDeleteQuery.h>
-#include <Parsers/ASTLiteral.h>
-#include <Parsers/CommonParsers.h>
-#include <Parsers/ParserDropQuery.h>
 #include <Storages/AlterCommands.h>
 #include <Storages/IStorage.h>
 #include <Storages/MutationCommands.h>
@@ -83,38 +80,6 @@ BlockIO InterpreterDeleteQuery::execute()
         return database->tryEnqueueReplicatedDDL(query_ptr, getContext(), {});
     }
 
-    /// DELETE FROM ... WHERE 1 should be executed as truncate
-    if ((table->supportsDelete() || table->supportsLightweightDelete()) && isAlwaysTruePredicate(delete_query.predicate))
-    {
-        auto context = getContext();
-        context->checkAccess(AccessType::TRUNCATE, table_id);
-        table->checkTableCanBeDropped(context);
-
-        TableExclusiveLockHolder table_excl_lock;
-        /// We don't need any lock for ReplicatedMergeTree and for simple MergeTree
-        /// For the rest of tables types exclusive lock is needed
-        if (!std::dynamic_pointer_cast<MergeTreeData>(table))
-            table_excl_lock = table->lockExclusively(context->getCurrentQueryId(), context->getSettingsRef()[Setting::lock_acquire_timeout]);
-
-        String truncate_query = "TRUNCATE TABLE " + table->getStorageID().getFullTableName()
-            + (delete_query.cluster.empty() ? "" : " ON CLUSTER " + backQuoteIfNeed(delete_query.cluster));
-
-        ParserDropQuery parser;
-        auto current_query_ptr = parseQuery(
-            parser,
-            truncate_query.data(),
-            truncate_query.data() + truncate_query.size(),
-            "ALTER query",
-            0,
-            DBMS_DEFAULT_MAX_PARSER_DEPTH,
-            DBMS_DEFAULT_MAX_PARSER_BACKTRACKS);
-
-        auto metadata_snapshot = table->getInMemoryMetadataPtr();
-        /// Drop table data, don't touch metadata
-        table->truncate(current_query_ptr, metadata_snapshot, context, table_excl_lock);
-        return {};
-    }
-
     auto table_lock = table->lockForShare(getContext()->getCurrentQueryId(), getContext()->getSettingsRef()[Setting::lock_acquire_timeout]);
     auto metadata_snapshot = table->getInMemoryMetadataPtr();
 

src/Parsers/CommonParsers.cpp

@@ -1,10 +1,8 @@
 #include <algorithm>
 #include <cctype>
-#include <Parsers/ASTLiteral.h>
 #include <Parsers/CommonParsers.h>
 #include <base/find_symbols.h>
 #include <Common/ErrorCodes.h>
-
 namespace DB
 {
 
@@ -156,20 +154,5 @@ bool ParserKeyword::parseImpl(Pos & pos, [[maybe_unused]] ASTPtr & node, Expecte
     return true;
 }
 
-bool isAlwaysTruePredicate(const ASTPtr & predicate)
-{
-    if (!predicate)
-        return true;
-
-    const auto * ast_literal = predicate->as<ASTLiteral>();
-    if (!ast_literal)
-        return false;
-
-    UInt64 result;
-    if (ast_literal->value.tryGet<UInt64>(result) && result == 1)
-        return true;
-    return false;
-}
-
 
 }

src/Parsers/CommonParsers.h

@@ -710,6 +710,4 @@ class ParserNothing : public IParserBase
     bool parseImpl(Pos & /*pos*/, ASTPtr & /*node*/, Expected & /*expected*/) override { return true; }
 };
 
-bool isAlwaysTruePredicate(const ASTPtr & predicate);
-
 }

tests/queries/0_stateless/02319_lightweight_delete_on_merge_tree_compact_parts.reference

@@ -5,6 +5,7 @@
 1	Compact
 95
 1
+1	Compact
 0
 1
 5	Compact

tests/queries/0_stateless/02994_merge_tree_mutations_cleanup.reference

@@ -1,4 +1,4 @@
-mutations after ALTER for data_rmt	0
-mutations after cleanup for data_rmt	0
-mutations after ALTER for data_mt	0
-mutations after cleanup for data_mt	0
+mutations after ALTER for data_rmt	1
+mutations after cleanup for data_rmt	1
+mutations after ALTER for data_mt	1
+mutations after cleanup for data_mt	1

tests/queries/0_stateless/03223_system_tables_set_not_ready.reference

@@ -1,5 +1,5 @@
 system.distribution_queue	1
 system.rocksdb	1
 system.databases	1
-system.mutations	0
+system.mutations	1
 system.replication_queue	1