Handle not implemented cases properly

src/Interpreters/InterpreterDropQuery.cpp

@@ -30,6 +30,7 @@ namespace ErrorCodes
     extern const int SYNTAX_ERROR;
     extern const int UNKNOWN_TABLE;
     extern const int UNKNOWN_DICTIONARY;
+    extern const int NOT_IMPLEMENTED;
 }
 
 
@@ -55,6 +56,8 @@ BlockIO InterpreterDropQuery::execute()
     {
         if (!drop.is_dictionary)
             return executeToTable(drop);
+        else if (drop.permanently && drop.kind == ASTDropQuery::Kind::Detach)
+            throw Exception("DETACH PERMANENTLY is not implemented for dictionaries", ErrorCodes::NOT_IMPLEMENTED);
         else
             return executeToDictionary(drop.database, drop.table, drop.kind, drop.if_exists, drop.temporary, drop.no_ddl_lock);
     }
@@ -296,11 +299,15 @@ BlockIO InterpreterDropQuery::executeToDatabaseImpl(const ASTDropQuery & query,
             bool drop = query.kind == ASTDropQuery::Kind::Drop;
             context.checkAccess(AccessType::DROP_DATABASE, database_name);
 
+            if (query.kind == ASTDropQuery::Kind::Detach && query.permanently)
+                throw Exception("DETACH PERMANENTLY is not implemented for databases", ErrorCodes::NOT_IMPLEMENTED);
+
 #if USE_MYSQL
             if (database->getEngineName() == "MaterializeMySQL")
                 stopDatabaseSynchronization(database);
 #endif
 
+
             if (database->shouldBeEmptyOnDetach())
             {
                 /// DETACH or DROP all tables and dictionaries inside database.

src/Parsers/ParserDropQuery.cpp

@@ -11,7 +11,7 @@ namespace DB
 namespace
 {
 
-bool parseDropQuery(IParser::Pos & pos, ASTPtr & node, Expected & expected, bool optional_table_keyword = false)
+bool parseDropQuery(IParser::Pos & pos, ASTPtr & node, Expected & expected, const ASTDropQuery::Kind kind)
 {
     ParserKeyword s_temporary("TEMPORARY");
     ParserKeyword s_table("TABLE");
@@ -42,15 +42,6 @@ bool parseDropQuery(IParser::Pos & pos, ASTPtr & node, Expected & expected, bool
 
         if (!name_p.parse(pos, database, expected))
             return false;
-
-        if (ParserKeyword{"ON"}.ignore(pos, expected))
-        {
-            if (!ASTQueryWithOnCluster::parse(pos, cluster_str, expected))
-                return false;
-        }
-
-        if (s_no_delay.ignore(pos, expected) || s_sync.ignore(pos, expected))
-            no_delay = true;
     }
     else
     {
@@ -61,7 +52,8 @@ bool parseDropQuery(IParser::Pos & pos, ASTPtr & node, Expected & expected, bool
         else if (s_temporary.ignore(pos, expected))
             temporary = true;
 
-        if (!is_view && !is_dictionary && (!s_table.ignore(pos, expected) && !optional_table_keyword))
+        /// for TRUNCATE queries TABLE keyword is assumed as default and can be skipped
+        if (!is_view && !is_dictionary && (!s_table.ignore(pos, expected) && kind == ASTDropQuery::Kind::Truncate))
         {
             return false;
         }
@@ -78,24 +70,26 @@ bool parseDropQuery(IParser::Pos & pos, ASTPtr & node, Expected & expected, bool
             if (!name_p.parse(pos, table, expected))
                 return false;
         }
-
-        if (ParserKeyword{"ON"}.ignore(pos, expected))
-        {
-            if (!ASTQueryWithOnCluster::parse(pos, cluster_str, expected))
-                return false;
-        }
-
-        if (s_permanently.ignore(pos, expected))
-            permanently = true;
-
-        if (s_no_delay.ignore(pos, expected) || s_sync.ignore(pos, expected))
-            no_delay = true;
     }
 
+    /// common for tables / dictionaries / databases
+    if (ParserKeyword{"ON"}.ignore(pos, expected))
+    {
+        if (!ASTQueryWithOnCluster::parse(pos, cluster_str, expected))
+            return false;
+    }
+
+    if (kind == ASTDropQuery::Kind::Detach && s_permanently.ignore(pos, expected))
+        permanently = true;
+
+    /// actually for TRUNCATE NO DELAY / SYNC means nothing
+    if (s_no_delay.ignore(pos, expected) || s_sync.ignore(pos, expected))
+        no_delay = true;
+
     auto query = std::make_shared<ASTDropQuery>();
     node = query;
 
-    query->kind = ASTDropQuery::Kind::Drop;
+    query->kind = kind;
     query->if_exists = if_exists;
     query->temporary = temporary;
     query->is_dictionary = is_dictionary;
@@ -111,28 +105,6 @@ bool parseDropQuery(IParser::Pos & pos, ASTPtr & node, Expected & expected, bool
     return true;
 }
 
-bool parseDetachQuery(IParser::Pos & pos, ASTPtr & node, Expected & expected)
-{
-    if (parseDropQuery(pos, node, expected))
-    {
-        auto * drop_query = node->as<ASTDropQuery>();
-        drop_query->kind = ASTDropQuery::Kind::Detach;
-        return true;
-    }
-    return false;
-}
-
-bool parseTruncateQuery(IParser::Pos & pos, ASTPtr & node, Expected & expected)
-{
-    if (parseDropQuery(pos, node, expected, true))
-    {
-        auto * drop_query = node->as<ASTDropQuery>();
-        drop_query->kind = ASTDropQuery::Kind::Truncate;
-        return true;
-    }
-    return false;
-}
-
 }
 
 bool ParserDropQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected)
@@ -142,11 +114,11 @@ bool ParserDropQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected)
     ParserKeyword s_truncate("TRUNCATE");
 
     if (s_drop.ignore(pos, expected))
-        return parseDropQuery(pos, node, expected);
+        return parseDropQuery(pos, node, expected, ASTDropQuery::Kind::Drop);
     else if (s_detach.ignore(pos, expected))
-        return parseDetachQuery(pos, node, expected);
+        return parseDropQuery(pos, node, expected, ASTDropQuery::Kind::Detach);
     else if (s_truncate.ignore(pos, expected))
-        return parseTruncateQuery(pos, node, expected);
+        return parseDropQuery(pos, node, expected, ASTDropQuery::Kind::Truncate);
     else
         return false;
 }

tests/queries/0_stateless/01600_detach_permanently.reference

@@ -1,4 +1,6 @@
-#####
+##################
+setup env
+##################
 db_ordinary.log_table 1
 >table detached!
 db_ordinary.log_table 2
@@ -12,7 +14,7 @@ db_ordinary.log_table 5
 >Table is back after attach
 db_ordinary.log_table 6
 10
-#####
+##################
 db_ordinary.mt_table 1
 >table detached!
 db_ordinary.mt_table 2
@@ -26,7 +28,7 @@ db_ordinary.mt_table 5
 >Table is back after attach
 db_ordinary.mt_table 6
 10
-#####
+##################
 db_ordinary.null_table 1
 >table detached!
 db_ordinary.null_table 2
@@ -40,7 +42,7 @@ db_ordinary.null_table 5
 >Table is back after attach
 db_ordinary.null_table 6
 0
-#####
+##################
 db_atomic.log_table 1
 >table detached!
 db_atomic.log_table 2
@@ -54,7 +56,7 @@ db_atomic.log_table 5
 >Table is back after attach
 db_atomic.log_table 6
 10
-#####
+##################
 db_atomic.mt_table 1
 >table detached!
 db_atomic.mt_table 2
@@ -68,7 +70,7 @@ db_atomic.mt_table 5
 >Table is back after attach
 db_atomic.mt_table 6
 10
-#####
+##################
 db_atomic.null_table 1
 >table detached!
 db_atomic.null_table 2
@@ -82,3 +84,19 @@ db_atomic.null_table 5
 >Table is back after attach
 db_atomic.null_table 6
 0
+##################
+test for MV
+MV is working
+Usual detach works immediately till restart
+Usual detach activates after restart
+Permanent detach works immediately
+Permanent detach still works after restart
+View can be reattached
+##################
+test for MV with inner table
+MV is working
+1
+View can be reattached
+##################
+DETACH DATABASE is not implemented (proper error)
+1

tests/queries/0_stateless/01600_detach_permanently.sh

@@ -6,10 +6,6 @@ CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
 ## tests with real clickhouse restart would be a bit to heavy,
 ## to ensure the table will not reappear back clickhose-local is enough.
 
-# TODO: clean it...
-# CLICKHOUSE_LOCAL="/home/mfilimonov/workspace/ClickHouse-detach-permanently/build/programs/clickhouse-local"
-# CLICKHOUSE_TMP=$(pwd)
-
 WORKING_FOLDER_01600="${CLICKHOUSE_TMP}/01600_detach_permanently"
 rm -rf "${WORKING_FOLDER_01600}"
 mkdir -p "${WORKING_FOLDER_01600}"
@@ -23,7 +19,7 @@ clickhouse_local() {
 test_detach_attach_sequence() {
     local db="$1"
     local table="$2"
-    echo "#####"
+    echo "##################"
 
     echo "${db}.${table} 1"
     # normal DETACH - while process is running (clickhouse-local here, same for server) table is detached.
@@ -49,6 +45,9 @@ test_detach_attach_sequence() {
     clickhouse_local "SELECT count() FROM ${db}.${table};"
 }
 
+echo "##################"
+echo "setup env"
+
 clickhouse_local "DROP DATABASE IF EXISTS db_ordinary SYNC;"
 clickhouse_local "DROP DATABASE IF EXISTS db_atomic SYNC;"
 
@@ -69,4 +68,58 @@ test_detach_attach_sequence "db_ordinary" "null_table"
 
 test_detach_attach_sequence "db_atomic" "log_table"
 test_detach_attach_sequence "db_atomic" "mt_table"
-test_detach_attach_sequence "db_atomic" "null_table"
+test_detach_attach_sequence "db_atomic" "null_table"
+
+echo "##################"
+echo "test for MV"
+clickhouse_local "CREATE TABLE db_ordinary.src Engine=Null AS system.numbers"
+clickhouse_local "CREATE TABLE db_ordinary.dst Engine=Log AS system.numbers"
+clickhouse_local "CREATE MATERIALIZED VIEW db_ordinary.src2dst_mv_to TO db_ordinary.dst AS SELECT * FROM db_ordinary.src"
+
+clickhouse_local "INSERT INTO db_ordinary.src SELECT * FROM numbers(10)"
+clickhouse_local "SELECT if(count() = 10, 'MV is working', 'MV failed') from db_ordinary.dst;"
+
+clickhouse_local "DETACH VIEW db_ordinary.src2dst_mv_to; INSERT INTO db_ordinary.src SELECT * FROM numbers(10)"
+clickhouse_local "SELECT if(count() = 10, 'Usual detach works immediately till restart', 'Usual detach failed') from db_ordinary.dst;"
+
+clickhouse_local "INSERT INTO db_ordinary.src SELECT * FROM numbers(10)"
+clickhouse_local "SELECT if(count() = 20, 'Usual detach activates after restart', 'Usual detach reactivation failed') from db_ordinary.dst;"
+
+clickhouse_local "DETACH VIEW db_ordinary.src2dst_mv_to PERMANENTLY; INSERT INTO db_ordinary.src SELECT * FROM numbers(10)"
+clickhouse_local "SELECT if(count() = 20, 'Permanent detach works immediately', 'Permanent detach failed') from db_ordinary.dst;"
+
+clickhouse_local "INSERT INTO db_ordinary.src SELECT * FROM numbers(10)"
+clickhouse_local "SELECT if(count() = 20, 'Permanent detach still works after restart', 'Permanent detach reactivated!') from db_ordinary.dst;"
+
+## Quite silly: ATTACH MATERIALIZED VIEW don't work with short syntax (w/o select), but i can attach it using ATTACH TABLE ...
+clickhouse_local "ATTACH TABLE db_ordinary.src2dst_mv_to"
+clickhouse_local "INSERT INTO db_ordinary.src SELECT * FROM numbers(10)"
+clickhouse_local "SELECT if(count() = 30, 'View can be reattached', 'can not reattach permanently detached view') from db_ordinary.dst;"
+
+clickhouse_local "DROP VIEW db_ordinary.src2dst_mv_to SYNC"
+
+echo "##################"
+echo "test for MV with inner table"
+clickhouse_local "CREATE MATERIALIZED VIEW db_ordinary.src_mv_with_inner Engine=Log AS SELECT * FROM db_ordinary.src"
+clickhouse_local "INSERT INTO db_ordinary.src SELECT * FROM numbers(10)"
+
+clickhouse_local "SELECT if(count() = 10, 'MV is working', 'MV failed') FROM db_ordinary.src_mv_with_inner"
+
+clickhouse_local "DETACH VIEW db_ordinary.src_mv_with_inner PERMANENTLY; INSERT INTO db_ordinary.src SELECT * FROM numbers(10)" --stacktrace
+clickhouse_local "SELECT if(count() = 10, 'MV can be detached permanently', 'MV detach failed') FROM db_ordinary.src_mv_with_inner" 2>&1 | grep -c "db_ordinary.src_mv_with_inner doesn't exist"
+## Quite silly: ATTACH MATERIALIZED VIEW don't work with short syntax (w/o select), but i can attach it using ATTACH TABLE ...
+clickhouse_local "ATTACH TABLE db_ordinary.src_mv_with_inner"
+clickhouse_local "INSERT INTO db_ordinary.src SELECT * FROM numbers(10)"
+clickhouse_local "SELECT if(count() = 20, 'View can be reattached', 'can not reattach permanently detached view') from db_ordinary.src_mv_with_inner;"
+
+## clickhouse_local can't work with dicts...
+# mkdir -p "${WORKING_FOLDER_01600}/user_files"
+# echo "1" > "${WORKING_FOLDER_01600}/user_files/dummy_dict.tsv"
+# clickhouse_local "DROP DICTIONARY db_ordinary.dummy; CREATE DICTIONARY db_ordinary.dummy (id UInt64) PRIMARY KEY id LAYOUT(FLAT()) SOURCE(FILE(path 'dummy_dict.tsv' format 'TabSeparated')) LIFETIME(MIN 1 MAX 10); DETACH DICTIONARY db_ordinary.dummy PERMANENTLY; SELECT dictGet('db_ordinary.dummy','val',toUInt64(1));"
+
+echo "##################"
+echo "DETACH DATABASE is not implemented (proper error)"
+clickhouse_local "DETACH DATABASE db_ordinary PERMANENTLY;" 2>&1 | grep -c 'DETACH PERMANENTLY is not implemented'
+
+# clean up
+rm -rf "${WORKING_FOLDER_01600}"