Skip to content

Don't use locale in to_string#2

Merged
al13n321 merged 1 commit intoClickHouse/0.16.0from
globale
Jul 3, 2025
Merged

Don't use locale in to_string#2
al13n321 merged 1 commit intoClickHouse/0.16.0from
globale

Conversation

@al13n321
Copy link
Member

@al13n321 al13n321 commented Jul 3, 2025

std::locale("C") doesn't work in clickhouse because we disallow localeconv libc function (in ClickHouse/base/harmful/harmful.c). Thrift does std::ostream::imbue(std::locale("C")) when formatting thrift structs as string, presumably to make formatting of floats independent of environment. This PR just replaces std::locale("C") with std::locale::classic(), which should be equivalent according to documentation (Returns a reference to the "C" locale), but doesn't call localeconv.

@al13n321 al13n321 assigned al13n321 and unassigned al13n321 Jul 3, 2025
@al13n321 al13n321 merged commit 327ed53 into ClickHouse/0.16.0 Jul 3, 2025
@al13n321 al13n321 deleted the globale branch July 3, 2025 22:49
azat added a commit to azat/ClickHouse that referenced this pull request Oct 29, 2025
Not always github allows to fetch via upstream repo, sometimes it gives
"fatal: Fetched in submodule path 'contrib/zstd', but it did not contain
X." error [1].

    fatal: transport 'file' not allowed
    fatal: Fetched in submodule path 'contrib/zstd', but it did not contain 437081852c396eeb61edb7d47044d8cad911885e. Direct fetching of that commit failed.

Refs:
- ClickHouse/zstd#2
- ClickHouse/thrift#2
azat added a commit to azat/ClickHouse that referenced this pull request Oct 29, 2025
Not always github allows to fetch via upstream repo, sometimes it gives
"fatal: Fetched in submodule path 'contrib/zstd', but it did not contain
X." error [1].

    fatal: transport 'file' not allowed
    fatal: Fetched in submodule path 'contrib/zstd', but it did not contain 437081852c396eeb61edb7d47044d8cad911885e. Direct fetching of that commit failed.

Refs:
- ClickHouse/zstd#2
- ClickHouse/thrift#2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant