Merge pull request ClickHouse#75771 from aalexfvk/fix_ttl_parsing_on_attach

Fix `allow_suspicious_ttl_expressions` misbehaviour

Author: CurtizJ

src/Storages/MergeTree/ReplicatedMergeTreeTableMetadata.cpp

@@ -347,7 +347,9 @@ void ReplicatedMergeTreeTableMetadata::checkEquals(const ReplicatedMergeTreeTabl
     }
 
     auto parsed_primary_key = KeyDescription::parse(primary_key, columns, context, true);
-    String parsed_zk_ttl_table = formattedAST(TTLTableDescription::parse(from_zk.ttl_table, columns, context, parsed_primary_key).definition_ast);
+    // Strict checking of suspicious TTL is not needed here
+    String parsed_zk_ttl_table = formattedAST(
+        TTLTableDescription::parse(from_zk.ttl_table, columns, context, parsed_primary_key, /* is_attach = */ true).definition_ast);
     if (ttl_table != parsed_zk_ttl_table)
     {
         throw Exception(ErrorCodes::METADATA_MISMATCH, "Existing table metadata in ZooKeeper differs in TTL. "

src/Storages/TTLDescription.cpp

@@ -436,7 +436,8 @@ TTLTableDescription TTLTableDescription::getTTLForTableFromAST(
     return result;
 }
 
-TTLTableDescription TTLTableDescription::parse(const String & str, const ColumnsDescription & columns, ContextPtr context, const KeyDescription & primary_key)
+TTLTableDescription TTLTableDescription::parse(
+    const String & str, const ColumnsDescription & columns, ContextPtr context, const KeyDescription & primary_key, bool is_attach)
 {
     TTLTableDescription result;
     if (str.empty())
@@ -446,7 +447,7 @@ TTLTableDescription TTLTableDescription::parse(const String & str, const Columns
     ASTPtr ast = parseQuery(parser, str, 0, DBMS_DEFAULT_MAX_PARSER_DEPTH, DBMS_DEFAULT_MAX_PARSER_BACKTRACKS);
     FunctionNameNormalizer::visit(ast.get());
 
-    return getTTLForTableFromAST(ast, columns, context, primary_key, context->getSettingsRef()[Setting::allow_suspicious_ttl_expressions]);
+    return getTTLForTableFromAST(ast, columns, context, primary_key, is_attach);
 }
 
 }

src/Storages/TTLDescription.h

@@ -139,7 +139,7 @@ struct TTLTableDescription
         const ASTPtr & definition_ast, const ColumnsDescription & columns, ContextPtr context, const KeyDescription & primary_key, bool is_attach);
 
     /// Parse description from string
-    static TTLTableDescription parse(const String & str, const ColumnsDescription & columns, ContextPtr context, const KeyDescription & primary_key);
+    static TTLTableDescription parse(const String & str, const ColumnsDescription & columns, ContextPtr context, const KeyDescription & primary_key, bool is_attach);
 };
 
 }

tests/queries/0_stateless/03352_allow_suspicious_ttl.reference

@@ -0,0 +1 @@
+CREATE TABLE default.replicated_ttl_00933\n(\n    `a` Int32 TTL now() + toIntervalDay(1),\n    `d` DateTime\n)\nENGINE = ReplicatedMergeTree(\'/clickhouse/tables/default/test_ttl_00933\', \'r1\')\nORDER BY d\nSETTINGS index_granularity = 8192

tests/queries/0_stateless/03352_allow_suspicious_ttl.sql

@@ -0,0 +1,58 @@
+ -- Tags: long, zookeeper
+
+-- Replicated
+
+SET allow_suspicious_ttl_expressions = 0;
+DROP TABLE IF EXISTS replicated_ttl_00933 SYNC;
+
+-- Create
+CREATE TABLE replicated_ttl_00933 (a Int32, d DateTime)
+  ENGINE=ReplicatedMergeTree('/clickhouse/tables/{database}/test_ttl_00933', 'r1')
+  ORDER BY a PARTITION BY tuple() TTL now() + INTERVAL 1 second;  -- { serverError BAD_ARGUMENTS }
+
+SET allow_suspicious_ttl_expressions = 1;
+
+CREATE TABLE replicated_ttl_00933 (a Int32, d DateTime)
+  ENGINE=ReplicatedMergeTree('/clickhouse/tables/{database}/test_ttl_00933', 'r1')
+  ORDER BY a PARTITION BY tuple() TTL now() + INTERVAL 1 second;
+
+-- Alter
+SET allow_suspicious_ttl_expressions = 0;
+ALTER TABLE replicated_ttl_00933 MODIFY TTL now() + interval 1 day; -- { serverError BAD_ARGUMENTS }
+
+SET allow_suspicious_ttl_expressions = 1;
+ALTER TABLE replicated_ttl_00933 MODIFY TTL now() + interval 1 day;
+
+DROP TABLE IF EXISTS replicated_ttl_00933 SYNC;
+
+-- Column TTL
+SET allow_suspicious_ttl_expressions = 0;
+
+CREATE TABLE replicated_ttl_00933 
+(
+    a Int32 TTL now() + INTERVAL 1 second,
+    d DateTime 
+)
+ENGINE=ReplicatedMergeTree('/clickhouse/tables/{database}/test_ttl_00933', 'r1')
+ORDER BY d;  -- { serverError BAD_ARGUMENTS }
+
+SET allow_suspicious_ttl_expressions = 1;
+
+CREATE TABLE replicated_ttl_00933 
+(
+    a Int32 TTL now() + INTERVAL 1 second,
+    d DateTime 
+)
+ENGINE=ReplicatedMergeTree('/clickhouse/tables/{database}/test_ttl_00933', 'r1')
+ORDER BY d;
+
+-- Alter column TTL
+SET allow_suspicious_ttl_expressions = 0;
+ALTER TABLE replicated_ttl_00933 MODIFY COLUMN a Int32 TTL now() + INTERVAL 1 day; -- { serverError BAD_ARGUMENTS }
+
+SET allow_suspicious_ttl_expressions = 1;
+ALTER TABLE replicated_ttl_00933 MODIFY COLUMN a Int32 TTL now() + INTERVAL 1 day;
+
+
+SHOW CREATE TABLE replicated_ttl_00933;
+DROP TABLE IF EXISTS replicated_ttl_00933 SYNC;