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.
Introduce "drivers" for executable user-defined functions. A driver is a
small adapter program declared in a server config under
`<user_defined_executable_function_drivers_config>` that knows how to take a
user-supplied code snippet and produce a runnable executable user-defined
function configuration.
New CREATE FUNCTION syntax:
CREATE [OR REPLACE] FUNCTION [IF NOT EXISTS] name [ON CLUSTER c]
ARGUMENTS (a UInt8, b String) RETURNS UInt64
ENGINE = DriverName(key1 = 'value1', key2 = 42)
AS '...code body...'
On CREATE FUNCTION, the corresponding `create_command` of the driver is
invoked: arguments are passed via `--name`, `--return`, `--args` and the
declared engine arguments; the body is sent on stdin; the driver writes the
resulting executable UDF configuration (XML or YAML) to stdout, which is
saved into the new `<dynamic_user_defined_executable_functions_path>` and
picked up by the existing executable UDF loader. The corresponding SQL is
persisted as `ATTACH FUNCTION ...` in `<user_defined>` and re-evaluated on
restart: if the dynamic configuration is still present, the driver is not
invoked; otherwise the driver is re-run to materialize it.
On DROP FUNCTION, the `drop_command` (if any) is invoked, then the dynamic
configuration file and per-function working directory are removed.
Includes a proof-of-concept driver `c_function_body`, which compiles a user
C body into a binary (inside Docker by default for build-time isolation) and
runs it inside Docker at query time with strict sandboxing (`--network=none`,
`--read-only`, dropped capabilities, no-new-privileges, unprivileged user,
resource limits) in `executable_pool` mode for batched per-row evaluation.
See https://github.com/ClickHouse/ClickHouse/issues/71172
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
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]>