Add GetString to ColumnDecimal.

[theory]

It converts the internal integer to a decimal string relative to the scale and precision. Not a blocker for my projects (I copied the code), but I am getting an error when using -D BUILD_SHARED_LIBS:

undefined symbol: _ZN4absllsERSoNS_6int128E

Presumably -D BUILD_SHARED_LIBS needs to also build and install the absl library.

[Copilot]

Pull request overview

This PR adds a GetString method to the ColumnDecimal class that converts internal integer representations to human-readable decimal strings with proper scale and precision formatting.

Key Changes:

• Added GetString method to ColumnDecimal class that formats decimal values as strings
• Added comprehensive test coverage for the new method across various decimal types and edge cases

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
clickhouse/columns/decimal.h	Declares the new GetString method with documentation
clickhouse/columns/decimal.cpp	Implements decimal-to-string conversion logic with proper handling of scale, precision, and negative values
ut/client_ut.cpp	Adds extensive test cases validating the string conversion for positive, negative, and edge-case decimal values

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[slabko]

I’m a bit on the fence about that. There’s a problem with this approach -- it’s five times slower than using SELECT toString(...) FROM ... and getting the same value. However, as soon as there is a GetString(...) function, it will become everyone’s default choice.

[theory]

I’m a bit on the fence about that. There’s a problem with this approach -- it’s five times slower than using SELECT toString(...) FROM ... and getting the same value. However, as soon as there is a GetString(...) function, it will become everyone’s default choice.

I'll look into whether I can use that syntax in pg_clickhouse, but ISTM that including stringification makes a more complete API.

Would it be more efficient to use a template to treat UIint32, UInt64, and UInt128 separately?

[theory]

I spent some time trying to use toString() in the queries built by pg_clickhouse to convert to strings on the back end. I got it working nicely for Postgres NUMERIC columns. For example, for this Postgres query:

SELECT * FROM dec_http.decimals ORDER BY id;

pg_clickhouse would send it as:

SELECT id, toString("dec"), toString(dec32), toString(dec64), toString(dec128)
FROM decimal_test.decimals ORDER BY id ASC NULLS LAST

But alas, the code is unaware of the data type of SQL expressions. So this Postgres query:

SELECT avg(dec) FROM dec_http.decimals;

Gets converted to:

SELECT avg(toString("dec")) FROM decimal_test.decimals

Which is obviously incorrect. It's far better to just get the Decimal value from a Block and to convert it to a string, as all the evaluation is done and ClickHouse has returned the proper value for the overall expression.

I assume I could figure out how to fiddle with the Postgres query AST enough at some point to work out where and how to do the casting, but it would take quite a bit of effort. At this point, I think it's simpler to just rely on what ClickHouse sends back. And for better or worse, to convert a Decimal to a Postgres NUMERIC, I need a string.

[slabko]

As noted earlier, if we introduce such a function, its performance should be on par with ClickHouse’s toString; a convenience API should not be several times slower than the next obvious alternative.

A more serious issue, however, is that this PR breaks shared-library builds. Running:

cmake -S . -B build -D BUILD_SHARED_LIBS=YES
cmake --build build

results in:

/usr/bin/ld: ../contrib/absl/absl/libabsl_int128.a(int128.cc.o): relocation R_X86_64_32S against symbol `_ZTVSt9basic_iosIcSt11char_traitsIcEE@@GLIBCXX_3.4' can not be used when making a shared object; recompile with -fPIC
/usr/bin/ld: failed to set dynamic section sizes: bad value
collect2: error: ld returned 1 exit status

[theory]

A more serious issue, however, is that this PR breaks shared-library builds

Does -fPIC help?

[slabko]

The lack of -fPIC is the reason it doesn’t compile. absl needs to be compiled with -fPIC in order to use operator<<. So yes, -fPIC does help.
See #454