Merge pull request #38590 from ClickHouse/proper_fix

Proper fix for ipv4/ipv6 conversion error

src/Databases/TablesLoader.cpp

@@ -171,6 +171,11 @@ void TablesLoader::removeUnresolvableDependencies(bool remove_loaded)
 
 void TablesLoader::loadTablesInTopologicalOrder(ThreadPool & pool)
 {
+    /// Compatibility setting which should be enabled by default on attach
+    /// Otherwise server will be unable to start for some old-format of IPv6/IPv4 types of columns
+    ContextMutablePtr load_context = Context::createCopy(global_context);
+    load_context->setSetting("cast_ipv4_ipv6_default_on_conversion_error", 1);
+
     /// Load independent tables in parallel.
     /// Then remove loaded tables from dependency graph, find tables/dictionaries that do not have unresolved dependencies anymore,
     /// move them to the list of independent tables and load.
@@ -183,7 +188,7 @@ void TablesLoader::loadTablesInTopologicalOrder(ThreadPool & pool)
         assert(metadata.parsed_tables.size() == tables_processed + metadata.independent_database_objects.size() + getNumberOfTablesWithDependencies());
         logDependencyGraph();
 
-        startLoadingIndependentTables(pool, level);
+        startLoadingIndependentTables(pool, level, load_context);
 
         TableNames new_independent_database_objects;
         for (const auto & table_name : metadata.independent_database_objects)
@@ -237,7 +242,7 @@ DependenciesInfosIter TablesLoader::removeResolvedDependency(const DependenciesI
     return metadata.dependencies_info.erase(info_it);
 }
 
-void TablesLoader::startLoadingIndependentTables(ThreadPool & pool, size_t level)
+void TablesLoader::startLoadingIndependentTables(ThreadPool & pool, size_t level, ContextMutablePtr load_context)
 {
     size_t total_tables = metadata.parsed_tables.size();
 
@@ -245,10 +250,10 @@ void TablesLoader::startLoadingIndependentTables(ThreadPool & pool, size_t level
 
     for (const auto & table_name : metadata.independent_database_objects)
     {
-        pool.scheduleOrThrowOnError([this, total_tables, &table_name]()
+        pool.scheduleOrThrowOnError([this, load_context, total_tables, &table_name]()
         {
             const auto & path_and_query = metadata.parsed_tables[table_name];
-            databases[table_name.database]->loadTableFromMetadata(global_context, path_and_query.path, table_name, path_and_query.ast, force_restore);
+            databases[table_name.database]->loadTableFromMetadata(load_context, path_and_query.path, table_name, path_and_query.ast, force_restore);
             logAboutProgress(log, ++tables_processed, total_tables, stopwatch);
         });
     }

src/Databases/TablesLoader.h

@@ -104,7 +104,7 @@ private:
 
     DependenciesInfosIter removeResolvedDependency(const DependenciesInfosIter & info_it, TableNames & independent_database_objects);
 
-    void startLoadingIndependentTables(ThreadPool & pool, size_t level);
+    void startLoadingIndependentTables(ThreadPool & pool, size_t level, ContextMutablePtr load_context);
 
     void checkCyclicDependencies() const;
 

src/Interpreters/InterpreterCreateQuery.cpp

@@ -1038,6 +1038,10 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create)
         create.attach = true;
         create.attach_short_syntax = true;
         create.if_not_exists = if_not_exists;
+
+        /// Compatibility setting which should be enabled by default on attach
+        /// Otherwise server will be unable to start for some old-format of IPv6/IPv4 types
+        getContext()->setSetting("cast_ipv4_ipv6_default_on_conversion_error", 1);
     }
 
     /// TODO throw exception if !create.attach_short_syntax && !create.attach_from_path && !internal

tests/integration/test_server_start_and_ip_conversions/__init__.py

@@ -0,0 +1 @@
+#!/usr/bin/env python3

tests/integration/test_server_start_and_ip_conversions/test.py

@@ -0,0 +1,58 @@
+#!/usr/bin/env python3
+import logging
+import pytest
+from helpers.cluster import ClickHouseCluster
+
+cluster = ClickHouseCluster(__file__)
+node = cluster.add_instance("node", stay_alive=True)
+
+
+@pytest.fixture(scope="module", autouse=True)
+def start_cluster():
+    try:
+        cluster.start()
+        yield cluster
+    finally:
+        cluster.shutdown()
+
+
+def test_restart_success_ipv4():
+    node.query(
+        """
+        CREATE TABLE ipv4_test
+        (
+            id UInt64,
+            value String
+        ) ENGINE=MergeTree ORDER BY id""",
+        settings={"cast_ipv4_ipv6_default_on_conversion_error": 1},
+    )
+
+    node.query(
+        "ALTER TABLE ipv4_test MODIFY COLUMN value IPv4 DEFAULT ''",
+        settings={"cast_ipv4_ipv6_default_on_conversion_error": 1},
+    )
+
+    node.restart_clickhouse()
+
+    assert node.query("SELECT 1") == "1\n"
+
+
+def test_restart_success_ipv6():
+    node.query(
+        """
+        CREATE TABLE ipv6_test
+        (
+            id UInt64,
+            value String
+        ) ENGINE=MergeTree ORDER BY id""",
+        settings={"cast_ipv4_ipv6_default_on_conversion_error": 1},
+    )
+
+    node.query(
+        "ALTER TABLE ipv6_test MODIFY COLUMN value IPv6 DEFAULT ''",
+        settings={"cast_ipv4_ipv6_default_on_conversion_error": 1},
+    )
+
+    node.restart_clickhouse()
+
+    assert node.query("SELECT 1") == "1\n"

tests/queries/0_stateless/02316_cast_to_ip_address_default_column.sql

@@ -1,4 +1,5 @@
 -- Tags: no-backward-compatibility-check
+-- TODO: remove no-backward-compatibility-check after new 22.6 release
 
 SET cast_ipv4_ipv6_default_on_conversion_error = 1;
 
@@ -11,6 +12,13 @@ CREATE TABLE ipv4_test
 
 ALTER TABLE ipv4_test MODIFY COLUMN value IPv4 DEFAULT '';
 
+SET cast_ipv4_ipv6_default_on_conversion_error = 0;
+
+DETACH TABLE ipv4_test;
+ATTACH TABLE ipv4_test;
+
+SET cast_ipv4_ipv6_default_on_conversion_error = 1;
+
 DROP TABLE ipv4_test;
 
 DROP TABLE IF EXISTS ipv6_test;
@@ -20,7 +28,15 @@ CREATE TABLE ipv6_test
     value String
 ) ENGINE=MergeTree ORDER BY id;
 
-ALTER TABLE ipv6_test MODIFY COLUMN value IPv4 DEFAULT '';
+ALTER TABLE ipv6_test MODIFY COLUMN value IPv6 DEFAULT '';
+
+SET cast_ipv4_ipv6_default_on_conversion_error = 0;
+
+DETACH TABLE ipv6_test;
+ATTACH TABLE ipv6_test;
+
+SET cast_ipv4_ipv6_default_on_conversion_error = 1;
+
 SELECT * FROM ipv6_test;
 
 DROP TABLE ipv6_test;