`clickhouse-local` hard-coded the temporary storage limit to `1 GiB`,
which is limiting when spilling to disk, e.g. during an external merge
sort with a low `max_bytes_before_external_sort`.
Honor the existing `max_temporary_data_on_disk_size` server setting
instead. Unlike the regular server, `clickhouse-local` stores temporary
data in the system temporary directory by default, which can be small
(and is sometimes backed by RAM), so the conservative `1 GiB` default is
kept when the setting is not specified. It can now be raised, or lifted
entirely with a value of `0`, via the configuration file.
Closes: https://github.com/ClickHouse/ClickHouse/issues/101697
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
New keeper-only system table with one row per on-disk Raft log file
(changelog_<from>_<to>.bin[.zstd]) tracked by the in-process Keeper.
Mirrors the plumbing of system.keeper_snapshots.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Address review: the shared `setupEchoAndHighlightSettings` made `--verbose`
imply query echoing for all clients, but historically only `clickhouse-local`
behaved that way. `clickhouse-client` and the embedded client defaulted
`echo_queries` from `--echo` only, so `clickhouse-client --verbose --query ...`
emitted just the result.
Make the implication opt-in via a `verbose_implies_echo` parameter (passed
`true` only from `LocalServer`), restoring the prior contract that existing
behavior stays identical unless the new echo options are used.
Extend `04308_echo_options_granular` to cover both cases: `--verbose` echoes
in `clickhouse-local` but not in `clickhouse-client`.
Implement https://github.com/ClickHouse/ClickHouse/issues/95479.
Previously, the client echoed queries differently depending on the mode
with no way to control it granularly: in interactive mode it always
printed a formatted and highlighted version of the query and the
`query_id`, while in batch mode `--echo` printed the raw query text only.
This change makes the behavior configurable while keeping the defaults
identical to before:
- `--echo` now takes an optional boolean value and controls echoing of
queries in both interactive and batch mode. It defaults to on in
interactive mode and off in batch mode.
- `--echo-formatted` is a new boolean option controlling whether the
echoed query is formatted. It defaults to on in interactive mode and
off in batch mode, so highlighting non-formatted queries is now
possible as well.
- `--echo-query-id` is a new boolean option controlling whether the
`query_id` is printed before execution. It defaults to on in
interactive mode and off in batch mode. When enabled in batch mode, a
fresh `query_id` is generated per query so that the printed value
matches the one used for execution.
- `--hilite` (`--highlight`) now controls highlighting of both the
command prompt and the echoed queries. Highlighting is only applied
when writing to a terminal.
The echoing logic for all modes is unified in the new `echoQuery`
method, and the per-client defaults are resolved in
`setupEchoAndHighlightSettings`.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
The constructor body of ThreadFromGlobalPoolImpl was a fully-inlined
lambda referencing DB::ThreadStatus and SCOPE_EXIT, which forced
ThreadPool.h to pull Common/ThreadStatus.h, Common/StackTrace.h and
base/scope_guard.h into ~329 TUs that include it transitively.
Hoist the nested State struct to a non-template ThreadFromGlobalPoolState
and move the lambda body into a free function startThreadFromGlobalPool
that lives in ThreadPool.cpp. The compile-time
global_trace_collector_allowed branch becomes a runtime bool (single
check per thread start, invisible cost).
Drop the now-unused includes of Common/ThreadStatus.h and
Common/StackTrace.h from ThreadPool.h.
A positional file argument is converted to --queries-file inside
ClientApplicationBaseParser via options.emplace, which populates the
boost options map (and the queries_files member) but never the Poco
config. The conflict check in processConfig used config.has("queries-file"),
so the positional case slipped through and the -q query was silently
dropped instead of erroring.
Check queries_files directly so both forms are rejected consistently.
Closes#104735
`createClickHouseLocalDatabaseOverlay` looked up the persisted database UUID
at `metadata/default` (hardcoded). `DatabaseAtomic` actually creates the
symlink at `metadata/<escapeForFileName(name)>`, so when the user passes
`--default_database=mydb` the lookup misses the previous run's symlink, a
fresh UUID is minted, and the previous run's data is orphaned. The table
then shows up as `UNKNOWN_TABLE` on the next invocation despite the data
still sitting in `store/`.
Use `DatabaseCatalog::getMetadataDirPath(name_)` instead, which is the same
helper that `DatabaseAtomic` uses for its metadata symlink path.
Closes#101831.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
* Make client rainbow parentheses optional
* Make client rainbow parentheses optional
* Make client rainbow parentheses optional
* Make client rainbow parentheses optional
* Fix style and clang-tidy CI failures
- In `programs/format/Format.cpp`, replace `options.count(...)` with
`options.contains(...)` to satisfy the `readability-container-contains`
clang-tidy check that fires in the `arm_tidy` build.
- In `src/Client/ClientBaseHelpers.cpp`, drop redundant braces around
single-statement `if (!rainbow_parentheses) ...` blocks. One of them
had the opening brace on the same line, which the C++ style check
rejects (Allman braces are required).
CI report: https://github.com/ClickHouse/ClickHouse/pull/103851
---------
Co-authored-by: Alexey Milovidov <[email protected]>
Add a server setting (`min_allocation_size_to_throw_on_memory_limit`) to
allow any allocation to throw `MEMORY_LIMIT_EXCEEDED`, this should help
with preventing OOMs
Right now, any `new` cannot throw, this is due exception safety issues,
and only explicitly marked can throw `MEMORY_LIMIT_EXCEEDED`.
But this can lead to real OOMs, i.e. when kernel will kill
clickhouse-server, even though we are trying hard to track memory
properly.
Add a server setting `min_allocation_size_to_throw_on_memory_limit`
(UInt64 bytes, default 0). When non-zero, the throwing `operator new`
variants are allowed to raise `MEMORY_LIMIT_EXCEEDED`. _The assumption
is that code with big enough allocations does not exception safety
issues._
Note, places that allows (Allocator)/never allows (malloc)
`MEMORY_LIMIT_EXCEEDED` before are not affected.
Addresses clickhouse-gh[bot] review on PR #105008 (id 3247356705).
Previously only `user_directories.users_xml.path` was rebased to the
loaded config's directory; the top-level `<users_config>` was passed
to `ConfigProcessor` verbatim and resolved against the current
working directory. So `~/.clickhouse-local/config.xml` with
`<users_config>users.xml</users_config>` could silently load an
unrelated `./users.xml` from cwd instead of failing fast when the
configured file is missing.
Move the relative-path rebase out of the `user_directories` branch so
it applies uniformly to both `users_config` and
`user_directories.users_xml.path`.
Extend `04245_clickhouse_local_home_config.sh` with a matching sub-case
for the `users_config` form: a `~/.clickhouse-local/config.xml` that
references a relative `users.xml` which is missing next to the config,
with a rogue `users.xml` planted in cwd, must surface
`FILE_DOESNT_EXIST` and not silently load the rogue file. Verified
locally: test fails on master HEAD without this commit, passes
50/50 with full randomization with it.
Also trims the prior verbose 5-line security comment to 3 lines, per
@Algunenano's earlier feedback on comment verbosity throughout the PR.
Previously the rebase to `config_dir` was guarded by an `fs::exists` check.
When the configured `users.xml` was missing next to the loaded config (for
example a stale path or a TOCTOU race where the file appears later), the
relative path was passed unchanged to `ConfigProcessor`, which resolves it
against the current working directory. A `users.xml` sitting in the cwd
would then be silently loaded instead of the one the user configured -
and that file could trivially grant `access_management` or otherwise widen
permissions for the `default` user.
Drop the `exists` guard and always rebase relative `users_xml.path` values
against the config's own directory. If the file is missing, `ConfigProcessor`
now throws `FILE_DOESNT_EXIST` with the configured absolute path, surfacing
a clear error instead of falling through to an unrelated file.
Verified locally: with `~/.clickhouse-local/config.xml` pointing at
`users.xml` and that file missing, the previous binary silently picked up
`./users.xml` from cwd (`max_threads = 99` in my repro); the fixed binary
throws `Configuration file <abs-path>/users.xml doesn't exist`. New
regression coverage in `04245_clickhouse_local_home_config.sh` asserts this
behavior.
Addresses clickhouse-gh[bot]'s review and @Algunenano's request to verify
the bot's claim on https://github.com/ClickHouse/ClickHouse/pull/105008.
`LocalServer::initialize` loads the configuration file from one of:
- `--config-file` command-line flag
- `./config.xml` in the current working directory
- `getLocalConfigPath` discovery: `./clickhouse-local.{xml,yaml,yml}`,
`~/.clickhouse-local/config.{xml,yaml,yml}`, `/etc/clickhouse-local/config.{xml,yaml,yml}`
But `LocalServer::setupUsers` only recognised the first two sources via its
gating condition:
if (getClientConfiguration().has("config-file") || fs::exists("config.xml"))
For configs discovered by `getLocalConfigPath` (the documented user-config path
introduced by #67135), the condition was false and `setupUsers` fell through to
`minimal_default_user_xml`, silently dropping the user's `profiles`, `users`,
and `quotas`. Setting `max_threads = 42` in `~/.clickhouse-local/users.xml`
under the default profile was ignored, and the built-in default was returned.
The fix tracks the actual loaded config path in `LocalServer::loaded_config_path`
during `initialize`, and uses it in `setupUsers` both as the gate and as the
source of `config_dir` (which resolves relative `user_directories.users_xml.path`
entries against the config's own directory rather than the current working
directory). All three search paths plus `--config-file` and `./config.xml` now
share the same user-config loading code path.
A new regression test `04245_clickhouse_local_home_config.sh` exercises the
`HOME/.clickhouse-local/config.xml` and `./clickhouse-local.xml` discovery paths
and verifies that `--config-file` and the no-config fallback still work.
Closes#104932
Move three `static` helpers in `LocalServer.cpp` into anonymous namespaces
(`createMemoryDatabaseIfNotExists`, `createClickHouseLocalDatabaseOverlay`,
`getConfigurationFromXMLString`) to match the file's existing convention.
In `QueryProfiler.cpp`, document why the `-Wdisabled-macro-expansion` pragma
spans the full `sigaction` setup: `sigaction`, `SA_SIGINFO`/`SA_RESTART`,
`sigemptyset`, and `sigaddset` are all macro-expanded names on supported
platforms, so the suppression must cover the entire block.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
- `programs/local/LocalServer.cpp`: replace file-level
`-Wdisabled-macro-expansion` with a narrow push/pop around the two
`fmt::print(stderr, ...)` lines that need it on musl.
- `src/AggregateFunctions/Moments.h`: drop the file-level
`-Wdouble-promotion` pragma. `CorrMoments::get` and the two
`getStandardError` helpers now use `std::sqrt`, and the `boost::math`
call sites use `double` literals so no implicit float→double
promotion remains. `cmath` is now included explicitly.
- `src/AggregateFunctions/AggregateFunctionGroupNumericIndexedVectorDataBSI.h`:
drop the file-level `-Wdouble-promotion` pragma — the file already
performs all float/int conversions through `static_cast`.
- `src/Columns/Collator.cpp`,
`src/Storages/System/StorageSystemUnicode.cpp`,
`src/Storages/System/StorageSystemStackTrace.cpp`,
`src/Functions/DivisionUtils.h`: keep the file-level pragma but add a
short comment explaining why the broad scope is needed (ICU's
`U_ICU_ENTRY_POINT_RENAME` wrapper macros, glibc's `errno` macro, and
the templated `apply` methods that mix `Float32`/`Float64` operands
throughout the file).
- `arrayAggregation.cpp`, `distanceTransposed.cpp`, `randDistribution.cpp`,
`toDecimalString.cpp`, `geohashesInBox.cpp`, `seriesPeriodDetectFFT.cpp`:
drop the file-wide `-Wdouble-promotion` pragma in favor of explicit
`static_cast` at the actual conversion sites or qualified `std::sqrt`.
- `KeeperDataDumper.cpp`, `LocalServer.cpp`,
`ChecksumForCompressedBlock.cpp`, `tcp_protocol_fuzzer.cpp`,
`Server.cpp`: replace `static` on free functions with anonymous
namespaces, matching the project preference Algunenano flagged.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
A default `clickhouse-local` invocation has no system log sections in the config.
Initializing `SystemLogs` regardless meant the unique pointer in `ContextSharedPart`
was written during construction and again during teardown, racing under TSan with a
concurrent read from a `BackgroundSchedulePool` worker that calls
`getBackgroundSchedulePoolLog` after a task finishes. The race surfaced as a
"Scraping system tables" failure in the `Stateless tests (amd_tsan, parallel, 1/2)`
job, since CI scrapes system tables via short-lived `clickhouse-local` processes.
Gate the call on the config actually containing at least one entry from
`LIST_OF_ALL_SYSTEM_LOGS`. The custom config used by `04093_system_log_alter_column`
declares `query_log` and `processors_profile_log`, so the regression test continues
to exercise the original fix; default invocations stop touching `system_logs`.
CI report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=102316&sha=665b5c743abc6d9c6ac9946eb60d70df95ce08c8&name_0=PR&name_1=Stateless%20tests%20%28amd_tsan%2C%20parallel%2C%201%2F2%29
PR: https://github.com/ClickHouse/ClickHouse/pull/102316
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
- musl build: `stderr` is defined as a recursive macro `#define stderr (stderr)`,
which triggers `-Wdisabled-macro-expansion` when used as a function argument.
Suppress the warning in `LocalServer.cpp` the same way it is already handled
in `FuzzLoop.cpp`.
- arm_fuzzers build: the `DEFINE_BINARY_PROTO_FUZZER` macro from
`libprotobuf-mutator` expands to definitions of `LLVMFuzzerCustomMutator`,
`LLVMFuzzerCustomCrossOver`, and `LLVMFuzzerTestOneInput` without preceding
prototypes, and the generated `out.cpp` defines `GenerateSentence` without
one either. Add `-Wno-missing-prototypes` to the compile flags for the
codegen fuzzer sources where we cannot easily add declarations.
CI report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=100976&sha=latest&name_0=PR
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
When `--no-system-tables` is set, the system database is not attached,
so initializing system logs would fail or violate the flag semantics.
Skip the call in that case, consistent with how system table attachment
is gated.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Add `initializeSystemLogs` call to `LocalServer` so that
`clickhouse-local` creates system log tables (like `query_log`,
`processors_profile_log`) when they are configured via `--config-file`.
Without a config, this is a no-op.
Rewrite the test to run entirely in `clickhouse-local` with a custom
config, avoiding ALTER on the shared server where materialized views
prevent column modifications.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
