From 1ea5e7333ad4c1b23145f0df767d0bcb667e14bb Mon Sep 17 00:00:00 2001 From: "David E. Wheeler" Date: Thu, 20 Nov 2025 16:11:22 -0500 Subject: [PATCH] Add support for the `Decimal` data type The mappings already work for the HTTP engine, although its inserts were limited to `float8`, now fixed to use proper numeric formatting. Add the mappings for the binary engine. Use strings to convert between the Postgres and ClickHouse formats to as to avoid differences in their implementations: ClickHouse stores data as `Int32`, `Int64`, or `Int128`, while Postgres...well, I have no idea what the internals are, but using the `numeric_in` and `numeric_out` functions avoid all issues. Conversion from `Decimal` values returned from ClickHouse requires converting its internal `Int128` into a string. The clickhouse-cpp library doesn't provide this functionality, [yet], so implement it here. Unfortunately, this approach doesn't work with clickhouse-cpp compiled into a dynamic library, presumably because it doesn't also compile and install the `absl` dynamic library. So make the build static by default on all platforms for now. Add a test that covers both the HTTP and binary engines. [yet]: https://github.com/ClickHouse/clickhouse-cpp/pull/451 --- Makefile | 4 +- src/binary.cpp | 75 +++++++++++++++++++++++++++ src/pglink.c | 9 ++-- test/expected/decimal.out | 104 ++++++++++++++++++++++++++++++++++++++ test/sql/decimal.sql | 46 +++++++++++++++++ 5 files changed, 230 insertions(+), 8 deletions(-) create mode 100644 test/expected/decimal.out create mode 100644 test/sql/decimal.sql diff --git a/Makefile b/Makefile index 65c41de..8ee725f 100644 --- a/Makefile +++ b/Makefile @@ -32,9 +32,9 @@ CH_CPP_FLAGS = -D CMAKE_BUILD_TYPE=Release -D WITH_OPENSSL=ON # Build static on Darwin by default. ifndef ($(CH_BUILD)) -ifeq ($(OS),Darwin) +# ifeq ($(OS),Darwin) CH_BUILD = static -endif +# endif endif # Are we statically compiling clickhouse-cpp into the extension or no? diff --git a/src/binary.cpp b/src/binary.cpp index d285d5a..ddcb449 100644 --- a/src/binary.cpp +++ b/src/binary.cpp @@ -1,3 +1,4 @@ +#include #include #include #include @@ -250,6 +251,11 @@ static Oid get_corr_postgres_type(const TypeRef & type) return FLOAT4OID; case Type::Code::Float64: return FLOAT8OID; + case Type::Code::Decimal128: + case Type::Code::Decimal64: + case Type::Code::Decimal32: + case Type::Code::Decimal: + return NUMERICOID; case Type::Code::FixedString: case Type::Code::Enum8: case Type::Code::Enum16: @@ -461,6 +467,26 @@ static void column_append(clickhouse::ColumnRef col, Datum val, Oid valtype, boo } break; } + case NUMERICOID: { + // Convert numeric to string and let ColumnDecimal parse it. + char *s = DatumGetCString(DirectFunctionCall1(numeric_out, val)); + switch (col->Type()->GetCode()) + { + case Type::Code::Decimal128: + case Type::Code::Decimal64: + case Type::Code::Decimal32: + case Type::Code::Decimal: + col->As()->Append(std::string(s)); + break; + default: + throw std::runtime_error( + "unexpected column " + "type for NUMERIC: " + + col->Type()->GetName()); + } + pfree(s); + break; + } case TEXTOID: { char * s = TextDatumGetCString(val); @@ -745,6 +771,55 @@ static Datum make_datum(clickhouse::ColumnRef col, size_t row, Oid * valtype, bo *valtype = FLOAT8OID; } break; + case Type::Code::Decimal128: + case Type::Code::Decimal64: + case Type::Code::Decimal32: + case Type::Code::Decimal: + { + auto decCol = col->As(); + auto val = decCol->At(row); + + // Convert the Int128 to a string. + std::stringstream ss; + ss << val; + std::string str = ss.str(); + + // Start a destination string. + std::stringstream res; + auto scale = decCol->GetScale(); + + // Output a dash for negative values + if (val < 0) + { + res << '-'; + str.erase(0, 1); + } + + if (str.length() <= scale) + { + // Append the entire value prepended with zeros after the decimal. + res << "0." << std::string(scale-1, '0') << str; + } + else + { + // There are digits before the decimal. + auto decAt = str.length() - scale; + res << str.substr(0, decAt); + + // Append any digits after the decimal. + if (decAt < str.length()) + { + res << '.' << str.substr(decAt); + } + } + + ret = DirectFunctionCall3(numeric_in, + CStringGetDatum(res.str().c_str()), + ObjectIdGetDatum(0), + Int32GetDatum(-1)); + *valtype = NUMERICOID; + } + break; case Type::Code::FixedString: { auto s = std::string(col->As()->At(row)); ret = CStringGetTextDatum(s.c_str()); diff --git a/src/pglink.c b/src/pglink.c index 550e543..4bd6bde 100644 --- a/src/pglink.c +++ b/src/pglink.c @@ -377,12 +377,9 @@ extend_insert_query(ch_http_insert_state *state, TupleTableSlot *slot) break; case NUMERICOID: { - Datum valueDatum; - float8 f; - - valueDatum = DirectFunctionCall1(numeric_float8, value); - f = DatumGetFloat8(valueDatum); - appendStringInfo(&state->sql, "%f", f); + char *extval = DatumGetCString(DirectFunctionCall1(numeric_out, value)); + appendStringInfoString(&state->sql, extval); + pfree(extval); } break; case BPCHAROID: diff --git a/test/expected/decimal.out b/test/expected/decimal.out new file mode 100644 index 0000000..69a786b --- /dev/null +++ b/test/expected/decimal.out @@ -0,0 +1,104 @@ +SET datestyle = 'ISO'; +CREATE SERVER binary_decimal_loopback FOREIGN DATA WRAPPER clickhouse_fdw OPTIONS(dbname 'decimal_test', driver 'binary'); +CREATE SERVER http_decimal_loopback FOREIGN DATA WRAPPER clickhouse_fdw OPTIONS(dbname 'decimal_test', driver 'http'); +CREATE USER MAPPING FOR CURRENT_USER SERVER binary_decimal_loopback; +CREATE USER MAPPING FOR CURRENT_USER SERVER http_decimal_loopback; +SELECT clickhouse_raw_query('DROP DATABASE IF EXISTS decimal_test'); + clickhouse_raw_query +---------------------- + +(1 row) + +SELECT clickhouse_raw_query('CREATE DATABASE decimal_test'); + clickhouse_raw_query +---------------------- + +(1 row) + +SELECT clickhouse_raw_query($$ + CREATE TABLE decimal_test.decimals ( + id Int32 NOT NULL, + dec Decimal(8, 0) NOT NULL, + dec32 Decimal32(4) NOT NULL, + dec64 Decimal64(6) NOT NULL, + dec128 Decimal128(8) NOT NULL + ) ENGINE = MergeTree PARTITION BY id ORDER BY (id); +$$); + clickhouse_raw_query +---------------------- + +(1 row) + +CREATE SCHEMA dec_bin; +CREATE SCHEMA dec_http; +IMPORT FOREIGN SCHEMA "decimal_test" FROM SERVER binary_decimal_loopback INTO dec_bin; +\d dec_bin.decimals + Foreign table "dec_bin.decimals" + Column | Type | Collation | Nullable | Default | FDW options +--------+---------------+-----------+----------+---------+------------- + id | integer | | not null | | + dec | numeric(8,0) | | not null | | + dec32 | numeric(9,4) | | not null | | + dec64 | numeric(18,6) | | not null | | + dec128 | numeric(38,8) | | not null | | +Server: binary_decimal_loopback +FDW options: (database 'decimal_test', table_name 'decimals', engine 'MergeTree') + +IMPORT FOREIGN SCHEMA "decimal_test" FROM SERVER http_decimal_loopback INTO dec_http; +\d dec_http.decimals + Foreign table "dec_http.decimals" + Column | Type | Collation | Nullable | Default | FDW options +--------+---------------+-----------+----------+---------+------------- + id | integer | | not null | | + dec | numeric(8,0) | | not null | | + dec32 | numeric(9,4) | | not null | | + dec64 | numeric(18,6) | | not null | | + dec128 | numeric(38,8) | | not null | | +Server: http_decimal_loopback +FDW options: (database 'decimal_test', table_name 'decimals', engine 'MergeTree') + +-- Fails pending https://github.com/ClickHouse/clickhouse-cpp/issues/422 +INSERT INTO dec_bin.decimals (id, dec, dec32, dec64, dec128) VALUES + (1, 42::NUMERIC, 98.6::NUMERIC, 102.4::NUMERIC, 1024.003::NUMERIC), + (2, 9999, 9999.9999, 9999999.999999, 99999999999.99999999), + (3, -9999, -9999.9999, -9999999.999999, -99999999999.99999999) +; +INSERT INTO dec_http.decimals VALUES + (4, 1000000::NUMERIC, 10000::NUMERIC, 3000000000::NUMERIC, 400000000000::NUMERIC), + (5, -1, -0.0001, -0.000001, -0.00000001), + (6, 0, 0, 0, 0) +; +SELECT * FROM dec_bin.decimals ORDER BY id; + id | dec | dec32 | dec64 | dec128 +----+---------+------------+-------------------+----------------------- + 1 | 42 | 98.6000 | 102.400000 | 1024.00300000 + 2 | 9999 | 9999.9999 | 9999999.999999 | 99999999999.99999999 + 3 | -9999 | -9999.9999 | -9999999.999999 | -99999999999.99999999 + 4 | 1000000 | 10000.0000 | 3000000000.000000 | 400000000000.00000000 + 5 | -1 | -0.0001 | -0.000001 | -0.00000001 + 6 | 0 | 0.0000 | 0.000000 | 0.00000000 +(6 rows) + +SELECT * FROM dec_http.decimals ORDER BY id; + id | dec | dec32 | dec64 | dec128 +----+---------+------------+-------------------+----------------------- + 1 | 42 | 98.6000 | 102.400000 | 1024.00300000 + 2 | 9999 | 9999.9999 | 9999999.999999 | 99999999999.99999999 + 3 | -9999 | -9999.9999 | -9999999.999999 | -99999999999.99999999 + 4 | 1000000 | 10000.0000 | 3000000000.000000 | 400000000000.00000000 + 5 | -1 | -0.0001 | -0.000001 | -0.00000001 + 6 | 0 | 0.0000 | 0.000000 | 0.00000000 +(6 rows) + +SELECT clickhouse_raw_query('DROP DATABASE decimal_test'); + clickhouse_raw_query +---------------------- + +(1 row) + +DROP USER MAPPING FOR CURRENT_USER SERVER binary_decimal_loopback; +DROP USER MAPPING FOR CURRENT_USER SERVER http_decimal_loopback; +DROP SERVER binary_decimal_loopback CASCADE; +NOTICE: drop cascades to foreign table dec_bin.decimals +DROP SERVER http_decimal_loopback CASCADE; +NOTICE: drop cascades to foreign table dec_http.decimals diff --git a/test/sql/decimal.sql b/test/sql/decimal.sql new file mode 100644 index 0000000..b13fcae --- /dev/null +++ b/test/sql/decimal.sql @@ -0,0 +1,46 @@ +SET datestyle = 'ISO'; +CREATE SERVER binary_decimal_loopback FOREIGN DATA WRAPPER clickhouse_fdw OPTIONS(dbname 'decimal_test', driver 'binary'); +CREATE SERVER http_decimal_loopback FOREIGN DATA WRAPPER clickhouse_fdw OPTIONS(dbname 'decimal_test', driver 'http'); +CREATE USER MAPPING FOR CURRENT_USER SERVER binary_decimal_loopback; +CREATE USER MAPPING FOR CURRENT_USER SERVER http_decimal_loopback; + +SELECT clickhouse_raw_query('DROP DATABASE IF EXISTS decimal_test'); +SELECT clickhouse_raw_query('CREATE DATABASE decimal_test'); +SELECT clickhouse_raw_query($$ + CREATE TABLE decimal_test.decimals ( + id Int32 NOT NULL, + dec Decimal(8, 0) NOT NULL, + dec32 Decimal32(4) NOT NULL, + dec64 Decimal64(6) NOT NULL, + dec128 Decimal128(8) NOT NULL + ) ENGINE = MergeTree PARTITION BY id ORDER BY (id); +$$); + +CREATE SCHEMA dec_bin; +CREATE SCHEMA dec_http; +IMPORT FOREIGN SCHEMA "decimal_test" FROM SERVER binary_decimal_loopback INTO dec_bin; +\d dec_bin.decimals +IMPORT FOREIGN SCHEMA "decimal_test" FROM SERVER http_decimal_loopback INTO dec_http; +\d dec_http.decimals + +-- Fails pending https://github.com/ClickHouse/clickhouse-cpp/issues/422 +INSERT INTO dec_bin.decimals (id, dec, dec32, dec64, dec128) VALUES + (1, 42::NUMERIC, 98.6::NUMERIC, 102.4::NUMERIC, 1024.003::NUMERIC), + (2, 9999, 9999.9999, 9999999.999999, 99999999999.99999999), + (3, -9999, -9999.9999, -9999999.999999, -99999999999.99999999) +; + +INSERT INTO dec_http.decimals VALUES + (4, 1000000::NUMERIC, 10000::NUMERIC, 3000000000::NUMERIC, 400000000000::NUMERIC), + (5, -1, -0.0001, -0.000001, -0.00000001), + (6, 0, 0, 0, 0) +; + +SELECT * FROM dec_bin.decimals ORDER BY id; +SELECT * FROM dec_http.decimals ORDER BY id; + +SELECT clickhouse_raw_query('DROP DATABASE decimal_test'); +DROP USER MAPPING FOR CURRENT_USER SERVER binary_decimal_loopback; +DROP USER MAPPING FOR CURRENT_USER SERVER http_decimal_loopback; +DROP SERVER binary_decimal_loopback CASCADE; +DROP SERVER http_decimal_loopback CASCADE;