Merge pull request ClickHouse#87668 from ClickHouse/backport/25.8/86480

Backport ClickHouse#86480 to 25.8: Fix detach/attach for postgres database engine tables with numeric/decimal columns

Author: clickhouse-gh[bot]

src/Databases/PostgreSQL/DatabasePostgreSQL.cpp

@@ -6,6 +6,7 @@
 
 #include <DataTypes/DataTypeNullable.h>
 #include <DataTypes/DataTypeArray.h>
+#include <DataTypes/DataTypesDecimal.h>
 #include <Storages/AlterCommands.h>
 #include <Storages/NamedCollectionsHelpers.h>
 #include <Storages/StoragePostgreSQL.h>
@@ -519,6 +520,9 @@ ASTPtr DatabasePostgreSQL::getColumnDeclaration(const DataTypePtr & data_type) c
     if (which.isDateTime64())
         return makeASTDataType("DateTime64", std::make_shared<ASTLiteral>(static_cast<UInt32>(6)));
 
+    if (which.isDecimal())
+        return makeASTDataType("Decimal", std::make_shared<ASTLiteral>(getDecimalPrecision(*data_type)), std::make_shared<ASTLiteral>(getDecimalScale(*data_type)));
+
     return makeASTDataType(data_type->getName());
 }
 

src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp

@@ -331,42 +331,11 @@ ASTPtr StorageMaterializedPostgreSQL::getColumnDeclaration(const DataTypePtr & d
     if (which.isArray())
         return makeASTDataType("Array", getColumnDeclaration(typeid_cast<const DataTypeArray *>(data_type.get())->getNestedType()));
 
-    /// getName() for decimal returns 'Decimal(precision, scale)', will get an error with it
-    if (which.isDecimal())
-    {
-        auto make_decimal_expression = [&](std::string type_name)
-        {
-            auto ast_expression = std::make_shared<ASTDataType>();
-
-            ast_expression->name = type_name;
-            ast_expression->arguments = std::make_shared<ASTExpressionList>();
-            ast_expression->arguments->children.emplace_back(std::make_shared<ASTLiteral>(getDecimalScale(*data_type)));
-
-            return ast_expression;
-        };
-
-        if (which.isDecimal32())
-            return make_decimal_expression("Decimal32");
-
-        if (which.isDecimal64())
-            return make_decimal_expression("Decimal64");
-
-        if (which.isDecimal128())
-            return make_decimal_expression("Decimal128");
-
-        if (which.isDecimal256())
-            return make_decimal_expression("Decimal256");
-    }
-
     if (which.isDateTime64())
-    {
-        auto ast_expression = std::make_shared<ASTDataType>();
+        return makeASTDataType("DateTime64", std::make_shared<ASTLiteral>(static_cast<UInt32>(6)));
 
-        ast_expression->name = "DateTime64";
-        ast_expression->arguments = std::make_shared<ASTExpressionList>();
-        ast_expression->arguments->children.emplace_back(std::make_shared<ASTLiteral>(static_cast<UInt32>(6)));
-        return ast_expression;
-    }
+    if (which.isDecimal())
+        return makeASTDataType("Decimal", std::make_shared<ASTLiteral>(getDecimalPrecision(*data_type)), std::make_shared<ASTLiteral>(getDecimalScale(*data_type)));
 
     return makeASTDataType(data_type->getName());
 }

tests/integration/test_postgresql_database_engine/test.py

@@ -403,6 +403,71 @@ def test_datetime(started_cluster):
     assert "DateTime64(6)" in node1.query("show create table pg.test")
 
 
+def test_numeric_detach_attach(started_cluster):
+    cursor = started_cluster.postgres_conn.cursor()
+    cursor.execute("DROP TABLE IF EXISTS test_table")
+    cursor.execute("""
+        CREATE TABLE test_table (
+            numeric_1 numeric NOT NULL,
+            numeric_2 numeric(10) NOT NULL,
+            numeric_3 numeric(10, 0) NOT NULL,
+            numeric_4 numeric(5, 2) NOT NULL,
+            numeric_5 numeric(10, 5) NOT NULL,
+            numeric_6 numeric(20, 10) NOT NULL,
+            numeric_7 numeric(50, 20) NOT NULL,
+            decimal_1 decimal NOT NULL,
+            decimal_2 decimal(10) NOT NULL,
+            decimal_3 decimal(10, 0) NOT NULL,
+            decimal_4 decimal(5, 2) NOT NULL,
+            decimal_5 decimal(10, 5) NOT NULL,
+            decimal_6 decimal(20, 10) NOT NULL,
+            decimal_7 decimal(50, 20) NOT NULL
+        )
+    """)
+
+    node1.query("DROP DATABASE IF EXISTS postgres_database")
+    node1.query(
+        "CREATE DATABASE postgres_database ENGINE = PostgreSQL(postgres1)"
+    )
+
+    expected_clickhouse_column_types = {
+        "numeric_1": "Decimal(38, 19)",
+        "numeric_2": "Decimal(10, 0)",
+        "numeric_3": "Decimal(10, 0)",
+        "numeric_4": "Decimal(5, 2)",
+        "numeric_5": "Decimal(10, 5)",
+        "numeric_6": "Decimal(20, 10)",
+        "numeric_7": "Decimal(50, 20)",
+        "decimal_1": "Decimal(38, 19)",
+        "decimal_2": "Decimal(10, 0)",
+        "decimal_3": "Decimal(10, 0)",
+        "decimal_4": "Decimal(5, 2)",
+        "decimal_5": "Decimal(10, 5)",
+        "decimal_6": "Decimal(20, 10)",
+        "decimal_7": "Decimal(50, 20)",
+    }
+
+    def get_actual_clickhouse_column_types():
+        res = node1.query(
+            "SELECT name, type FROM system.columns WHERE database = 'postgres_database' AND table = 'test_table'"
+        )
+
+        return dict(line.split('\t') for line in res.splitlines())
+
+    assert get_actual_clickhouse_column_types() == expected_clickhouse_column_types
+
+    create_ddl = node1.query("SHOW CREATE TABLE postgres_database.test_table")
+    for column, expected_type in expected_clickhouse_column_types.items():
+        assert f"`{column}` {expected_type}" in create_ddl
+
+    node1.query("DETACH TABLE postgres_database.test_table")
+    node1.query("ATTACH TABLE postgres_database.test_table")
+
+    assert get_actual_clickhouse_column_types() == expected_clickhouse_column_types
+
+    node1.query("DROP DATABASE postgres_database")
+    cursor.execute(f"DROP TABLE test_table")
+
 def test_postgresql_password_leak(started_cluster):
     conn = get_postgres_conn(
         started_cluster.postgres_ip, started_cluster.postgres_port, database=True