The earlier server-side approach could not distinguish a compatibility-synthesized
setting from a genuine explicit override (both arrive as identical changes), so it
broke when the client's `compatibility` matched the profile's pinned version (the
reverted value was then the only setting transmitted, with no `compatibility` in the
batch to explain it) and it silently dropped genuine HTTP/SQL overrides.
Fix it on the client instead: the native client resets settings whose value was set
only by `compatibility` before transmitting them, so the server re-derives them from
`compatibility` itself and honors its own constraints (for example a profile pinning a
setting read-only). Server-to-server settings propagation is left unchanged. The
server-side `SettingsConstraints` heuristic is reverted.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Addresses review feedback on the native reader: when a `List`/`Map`/
`Struct`/`FixedSizeList` is empty, the referenced child range is empty,
so the child subtree must also be empty. A buffer-less child type such
as a `Null` field derives its size from the `FieldNode` length alone, so
a forged-huge child length in a tiny message could drive an arbitrary
allocation before the post-decode size check. Peek the child `FieldNode`
length and reject a non-zero value before `decodeField`.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
A native-protocol client resolves `compatibility` on its own side and then
transmits every reverted setting as an explicit change. For a setting a profile
pins with a `readonly` (CONST) constraint this failed the whole query with
SETTING_CONSTRAINT_VIOLATION, even though the server keeps the pinned value
regardless. The HTTP interface, which resolves `compatibility` server-side, was
never affected.
When a settings batch contains `compatibility`, reproduce the values it derives
and drop a change to a CONST-pinned setting when it merely carries that derived
value. A value differing from the derived one is a genuine override attempt of a
read-only setting and still fails, so the lockdown guarantee is preserved.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Apache Arrow Java < 19.0.0 emits a 0-byte offsets buffer for an empty
nested `List`/`Map` (a batch where every outer collection is empty).
Both the native `RecordBatchDecoder` and the Apache Arrow library
reader (`ArrowColumnToCHColumn`) rejected it with `INCORRECT_DATA`;
skip the offset validation when the list/map is empty, matching
arrow-cpp/pyarrow/arrow-rs. The buffer-size checks still run before any
allocation for non-empty lists/maps.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
check_fatal_messages_in_logs detected a <Fatal> by globbing all
clickhouse-server*.log, but then parsed a single log chosen by mtime
(pick_latest_file). On multi-server runs (e.g. DatabaseReplicated) the
crashed server stops logging first and so has the oldest mtime, while the
surviving replicas keep writing. Picking by mtime handed the FuzzerLogParser
a replica log with no <Fatal>, which matched no error pattern and produced a
useless "Unknown error" / "Lost connection to server" failure instead of
naming the crash.
Select the log that actually contains the <Fatal> (grep -l on content),
falling back to mtime only if no file matches.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Guards the iterative Field copy/destroy against regressions and covers the
container-lvalue entry points (the templated Field(T&&) constructor and
assignment, e.g. Field(object) in the analyzer): they forward to the underlying
container copy, whose elements are copied via the now-iterative Field copy, so a
deeply nested container lvalue must not overflow the native stack either.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The iterative destroy runs from `~Field` (noexcept), but its worklist can
allocate, and the global `operator new` throws `MEMORY_LIMIT_EXCEEDED` on the
soft memory limit like any other allocation. The old recursive destructor never
allocated, so this would be a new way for teardown to `std::terminate` under
memory pressure.
Wrap the worklist in `LockMemoryExceptionInThread`, which suppresses the
memory-limit exception while still tracking memory, so the memory freed during
teardown is still credited (no tracker drift) and only a genuine out-of-memory
`bad_alloc` remains. Also use an inline worklist buffer so the common (deep and
therefore narrow) case does not allocate at all.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
get_profiles dumped `select *` from system.query_log for both servers
(~120 MiB each, 85 columns), parsed downstream by analyze_queries and
report(). Only six columns are actually read by the report (query_id,
ProfileEvents, query_duration_ms, memory_usage, read_bytes,
written_bytes); the rest are only ever looked at by a human debugging a
run.
Dump a curated set instead: the six report columns plus the ones useful
for manual debugging (query text, Settings, timings, row/byte counts,
exceptions). This drops ~70 niche columns (initial_query_*, transaction_id,
thread_ids, used_*, client_*, microsecond-precision duplicates, ...) and
shrinks each dump from ~120 MiB to ~92 MiB, so the artifact is smaller and
analyze/report parse less. ProfileEvents and Settings, the two large
columns, are kept.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
eqmed.sql runs a randomization test for the median difference between the
two versions. It cross-joined each query's measurements with
numbers(1, 10000) and did `order by rand()`, so a query with 1000
measurements built and sorted 10M rows in memory: ~22 GiB peak and ~15s,
single-threaded. That 22 GiB spike is why the analyze stage caps
concurrency at MemTotal/15.
Two changes, same statistic (10000 iterations, exact medians):
- Keep each query's measurements as one array and reshuffle it per
iteration (arrayShuffle + arrayReduce('medianExactForEach', ...) on each
half), with max_block_size bounding how many shuffled copies exist at
once, instead of materializing and sorting numbers*T rows.
- Only randomize metrics that actually vary across the measurements. A
constant metric has identical medians in every split, so its threshold
is exactly 0 (which the old query also produced); most of the ~230
ProfileEvents are constant per query. The threshold is scattered back to
full width with 0 for the constant metrics.
Verified on 505 real query inputs from a CI run: the deterministic
columns (left/right medians, diff) are byte-identical, and the randomized
threshold is statistically identical (median ratio 1.0; the only
differences are Monte-Carlo boundary flips the old query shows run to run
as well). On the heaviest query (1000 measurements) peak memory drops
from 22.5 GiB to 788 MiB and time from 15.4s to 2.48s; total CPU over all
505 queries drops from 896s to 225s.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
readQueryParameters used NameToNameMap::emplace, which keeps the first
occurrence of a duplicate key. The Settings-based path it replaced kept
the last occurrence (BaseSettings::read overwrites the same custom
setting entry on each occurrence of its name), so a driver relying on
appending an override without deduplicating first would see different
behavior. Use insert_or_assign to keep the old semantics, and add gtest
coverage for the round trip and the duplicate-name case.
Query parameters were transported by packing them into a Settings object,
so a parameter whose name collided with a builtin setting or alias
(limit, offset, max_threads, log_comment, ...) got value-normalized,
alias-resolved, or misquoted, poisoning every later SET param_* in the
session. Bypass Settings::set/read and the builtin-setting accessor
entirely for parameters, keeping them on the existing custom-setting wire
encoding. No protocol version bump required.
Closes: https://github.com/ClickHouse/ClickHouse/issues/85768
Closes: https://github.com/ClickHouse/ClickHouse/issues/91168
Related: https://github.com/ClickHouse/ClickHouse/pull/109526
Only the hits datasets are re-inserted; tpch, tpcds and values still read
their frozen tarball parts, so write-time defaults are not exercised on
those workloads. Make that scope explicit in populate_data.
Co-Authored-By: Claude Fable 5 <[email protected]>
rebuild_table treated any missing source as a benign skip, so if an
in-place dataset (default.hits_10m_single / default.hits_100m_single)
failed to attach, populate_data still wrote test._populate_done and every
later restart skipped the rebuild, leaving that dataset in the frozen
tarball format. Only the datasets.hits_v1 -> test.hits retry (source
already consumed into an existing destination) is a legitimate skip; every
other missing source now raises so the job fails instead of silently
marking an unrebuilt dataset as done.
Co-Authored-By: Claude Fable 5 <[email protected]>
Both fallback paths could hang the whole report on a wedged zstd:
- fetch_perf_report.py: proc.stderr.read() blocks until the child exits,
so proc.wait(timeout=120) was never reached. Replace it with a
threading.Timer watchdog that kills the child, which unblocks both the
stdin write and the stderr read and turns a stuck shard into the normal
per-shard error. Add a fast regression test using a fake, wedged zstd on
PATH and a shrunk timeout.
- fetch_ci_report.js: add a 120s timeout to the execFileSync zstd
fallback so a wedged child can no longer block the helper indefinitely.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
A nullptr result for a non-zero size means realloc failed and ptr is still live; untracking it there double-counted against the later free. Only untrack when realloc consumed ptr (success, or size==0 zero_realloc:free).
trackedZoneRealloc now charges only when the result is jemalloc-owned and untracks the old block only when it was owned, so foreign pointers (system realloc) and realloc(ptr,0) stay balanced. Document the accepted, measured per-thread TLV under-count limitation.
Darwin routes foreign (non-jemalloc) pointers through the default zone's free/realloc (jemalloc guards these with ivsalloc; see setenv). je_sallocx is UB on such pointers; use saved_zone.size, which returns 0 for pointers jemalloc doesn't own.
A deeply nested collection literal (e.g. `[[[ ... ]]]`) is parsed into a
single `Field` nested one `Array`/`Tuple`/`Map` per level. Since the collection
parser became iterative, that `Field` can be built to an arbitrary depth
(bounded only by the client-settable `max_parser_depth`) without tripping the
parser's periodic `checkStackSize` guard, which relies on native recursion to
fire.
The recursive `Field` copy constructor (reached via `ASTLiteral::clone` when the
analyzer clones the AST) and the `Field` destructor then recurse once per
nesting level with no stack guard and overflow the native stack, crashing the
server. A `checkStackSize` guard cannot fix the destructor because throwing from
a destructor calls `std::terminate`.
Make the copy and destroy of `Array`/`Tuple`/`Map`/`Object` fields (plus the
copy- and move-assignment paths that route through them) iterative, using an
explicit worklist so the native stack depth stays bounded regardless of the
nesting depth. Guarded type-inference/formatting walks still reject genuinely
too-deep structures cleanly with `TOO_DEEP_RECURSION`.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
jemalloc's zone is its own mutable static, so the page is always writable; the mprotect guarded an impossible case and its failure mode would have crashed startup.
Rebuilding the three tables in parallel on one server OOMed: the inserts
share the per-user memory limit (~28GiB) and hits_100m_single alone uses
~21GiB, so the OvercommitTracker killed the concurrent test.hits insert.
Go back to sequential rebuilds (the two servers are still populated in
parallel, which fits since each has its own user memory tracker).
Also stop using test.hits existence as the "done" marker: with rebuilds it
was not a reliable signal (the in-place *_single tables exist from the
tarball before being rebuilt). Create a dedicated test._populate_done
marker only after all three tables are rebuilt, and drop any leftover
target at the start of rebuild_table, so a retry after a mid-run failure
re-runs the whole populate instead of skipping it.
Co-Authored-By: Claude Fable 5 <[email protected]>
realloc(ptr,0) under jemalloc zero_realloc:free frees ptr and returns null; untrack the old block instead of leaking it. valloc is page-aligned, so charge the aligned size class to balance je_sallocx on free.
Wrap jemalloc's default malloc-zone callbacks so raw malloc/free (e.g. from croaring) is charged to the MemoryTracker on macOS, the zone-safe analog of the Linux symbol interposition. Unskips 01961_roaring_memory_tracking.
The reader selects a branch on a FlatBuffers union discriminant
(`Message.header_type`, `Field.type_type`) and then dereferences the matching
typed accessor (`header_as_X`, `type_as_X`). FlatBuffers verification only
proves that a union value, if present, is a well-formed table; these unions are
not `(required)`, so `VerifyMessageBuffer` accepts a buffer whose discriminant
is set while the value offset is absent (`Verifier::VerifyTable` returns true
for a null table). The typed accessor then returns null and the reader
dereferenced it, a remotely triggerable SIGSEGV reachable at schema-inference
time on any `file()`/`url()`/`s3()` Arrow read.
Guard both unions once at their choke points, `MessageReader::readNextMessage`
and `parseType`, and reject the missing value as `INCORRECT_DATA`.
Co-Authored-By: Claude Fable 5 <[email protected]>
Each server rebuilt its three hits tables sequentially, so the two small
tables (~20s each) added on top of hits_100m_single (~13min). Rebuild all
three in parallel instead: the small tables finish within the big table's
window, saving ~40s per shard. All tables still get OPTIMIZE FINAL to a
single part, so no background merges run during the tests.
Co-Authored-By: Claude Fable 5 <[email protected]>
The streaming zstd CLI fallback called proc.communicate() after manually
closing proc.stdin. communicate() then flushes the already-closed stdin,
which raises ValueError on Python 3.9-3.11 (the versions CI runs), so the
first valid .zst shard failed and compressed perf shards kept vanishing
from the report.
Stream the body in, close stdin (tolerating BrokenPipeError when zstd
exits early on corrupt input), read stderr, then wait() on the child
directly. Add a regression test that forces the CLI path by hiding the
zstandard module.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Address review on the compressed-artifact handling:
- fetch_perf_report.py: download_url streamed the whole shard body into
memory (resp.read() then write). Compressed perf artifacts exist only
for the large shards, and up to 8 shards download in parallel, so this
could hold hundreds of MiB. Stream the response through the matching
decompressor into dest instead, peeking the magic bytes via a small
_PrefixedReader.
- fetch_ci_report.js: maybeDecompress could throw from the async 'end'
handler (missing zstd binary, corrupt archive), escaping as an uncaught
exception. Wrap it in try/catch and reject so it surfaces normally.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Convert the test from .sql (no-parallel) to .sh so the global SQL UDF
name can embed $CLICKHOUSE_DATABASE, avoiding collisions with concurrent
runs. Same pattern as 04516.
The hits datasets are attached in Ordinary databases (default, datasets),
and EXCHANGE TABLES is only supported in Atomic databases, so the in-place
rebuild of hits_10m_single / hits_100m_single failed with NOT_IMPLEMENTED.
Swap the freshly-built table in with a two-table RENAME, which works in
Ordinary databases too.
Co-Authored-By: Claude Fable 5 <[email protected]>
The current projection metadata is not a valid fallback: projection parts
can legitimately lag behind it after an ALTER (readable through alter
conversions). Test the lagging case with a SELECT * projection plus
ALTER ADD COLUMN, which changes the projection metadata without creating
a mutation.
Co-Authored-By: Claude Fable 5 <[email protected]>
download_url only swallowed RuntimeError, but the decompression/fallback
paths can raise URLError, TimeoutExpired, gzip.BadGzipFile or
FileNotFoundError (e.g. when zstd is unavailable). Those escaped the
worker and aborted the whole report instead of warning on one shard and
continuing.
Let download_url raise, and have the download_shard worker catch any
shard-local download/decompress failure into the existing per-shard
error tuple. Add a regression test covering those exception types.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
A projection part that fails to load before its columns are set (for example
because of a corrupted serialization.json) has an empty column list. CHECK TABLE
compared the projection's on-disk columns.txt against that empty list and
reported a misleading "Columns doesn't match ... Expected: 0 columns" error,
hiding the real corruption. Take the expected columns from the projection
metadata when the part's own columns are unavailable, so the check proceeds and
reports the actual broken file. Extracted from #93694.
Co-Authored-By: Claude Fable 5 <[email protected]>
Removing the preconfig server also removed the side effect that created
`ci/tmp/coordination0`, which the configure step copied into each server's
keeper storage, so `cp` failed. Create each server's coordination dir
directly instead; the embedded, non-replicated keeper bootstraps from an
empty dir.
Co-Authored-By: Claude Fable 5 <[email protected]>
Address review feedback: a guard can miss a construct that lives in a
different node shape entirely, not only a different name. arrayJoin is
the case: the function form is ASTFunction/FunctionNode, but the
ARRAY JOIN clause is ASTArrayJoin/ArrayJoinNode, so banning only the
function still leaves the clause open.
The trigger now tells the reviewer to first enumerate every AST and
query-tree representation of the construct, then apply the alias, UDF,
and alternate-spelling checks to each name-based carrier.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
A guard that matches a SQL construct by name (a function, keyword, or
operator) misses the same semantics reached through other forms. This
is what PR 109753 hit: it rejected arrayJoin in row policy filters by
literal name, then PR 109973 had to follow up for the unnest alias and
for arrayJoin hidden inside a SQL UDF body.
Add a TRIGGERED EXPANSIONS entry so reviews of such guards check name
aliases (canonical-name resolution), user-defined expansions (SQL UDF
bodies, macro/lambda/view inlining), and alternate spellings. It covers
both the raw AST and the analyzer query tree, and calls out that the
guard must run after expansions have happened.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
When every matching perf job was intentionally SKIPPED (e.g. `--arch amd`
on a non-'pr-performance' PR) the tool exited 1, so automation could not
tell "no perf data" from a real failure. And the `clickhouse` PATH check
ran at the top of `main`, aborting on hosts without `clickhouse` even
when there was nothing to analyze.
Exit 0 in the all-skipped case, and require `clickhouse` only after
establishing there is at least one shard to download and classify.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The CI performance job runs `ci/jobs/performance_tests.py`, which does its
own dataset setup and only calls `compare.sh` for the report stage. The
previous commit changed `compare.sh`'s `configure`/`restart`, which the CI
never executes, so the re-insert never ran. Revert that dead change and
implement the same logic in `performance_tests.py`.
The configure step no longer starts a preconfig server to rename
`datasets.hits_v1` to `test.hits`; the datasets stay attached as-is. After
both servers start, each re-inserts its hits datasets and runs
`OPTIMIZE ... FINAL` to collapse them back to a single part matching the
original layout. Parts are written by that server's own binary and
settings, so a PR changing a write-time default is reflected on the tested
(right) side. Both servers are populated in parallel.
Co-Authored-By: Claude Fable 5 <[email protected]>
The performance-comparison setup attached the dataset tarballs directly,
so the hits tables kept their frozen on-disk format: non-adaptive marks,
no sparse serialization, no statistics, and a single pre-merged part.
Changes to write-time defaults (for example sparse columns or
`compute_exact_num_defaults_for_sparse_columns`) were therefore never
exercised by the performance tests.
Each server now re-inserts the attached datasets into its final tables
on startup, then runs `OPTIMIZE ... FINAL` to collapse them back to a
single part matching the original layout. The parts are written by that
server's own binary and settings, so a PR that changes a write-time
default is reflected on the tested (right) side. The left (reference) and
right servers are populated independently and in parallel.
Co-Authored-By: Claude Fable 5 <[email protected]>
The amd perf comparison only runs on 'pr-performance' PRs, so on a normal
PR those shards are SKIPPED and produce no artifacts. The tool tried to
download them anyway and printed six "Failed to download" warnings.
Detect shards whose result status is SKIPPED, print a single clean
"Skipped (<reason>)" line each, and exclude them from the download set.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
CI compresses uploaded text artifacts larger than a threshold with zstd
(ci/praktika/s3.py, COMPRESS_THRESHOLD_MB), so the same object (e.g. a
perf shard's all-query-metrics.tsv) can be served either plain or as a
`.zst` sibling. The report tools decoded the bytes as UTF-8 and failed on
the zstd magic byte, silently dropping the larger shards.
Make every artifact reader handle plain, zstd, and gzip transparently:
detect by magic bytes and, for downloads, fall back to the `.zst` sibling
when the plain object is 403/404.
- fetch_perf_report.py: maybe_decompress + `.zst` fallback in the fetch
and download paths.
- fetch_ci_report.js: decompress the response body by magic; retry `.zst`
on 403/404 (zlib.zstdDecompressSync, with a zstd-CLI fallback).
- perf_api.py: read local `.zst`/`.gz` TSVs transparently.
- Regression tests: plain vs gzip vs zstd parse identically.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The guard resolved aliases but still inspected only the surface AST, so a
SQL UDF whose body expands to arrayJoin/unnest (e.g.
CREATE FUNCTION f AS x -> unnest([1,2]) OR x = 0; ... USING f(value))
passed the check and reintroduced the column->size() == num_rows logical
error, because the UDF is inlined when the filter is built at read time.
Descend into SQL UDF bodies (UserDefinedSQLFunctionFactory::tryGet, with
cycle protection for nested/recursive UDFs) so an arrayJoin reachable
through any number of UDF wrappers is detected, at both CREATE/ALTER and
the query-apply guard.
The regression test now also covers a UDF wrapper; it creates a global SQL
UDF, so it is tagged no-parallel.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The guard matched the literal function name "arrayJoin" and missed its
case-insensitive alias unnest, so a row policy USING unnest([...]) was
still accepted and reintroduced the 'column->size() == num_rows' logical
error at read time.
Resolve the function name to its canonical form with
getFunctionCanonicalNameIfAny, so any spelling of unnest (and any future
case-insensitive alias of arrayJoin) is caught. The guard runs both at
CREATE/ALTER and at the query-apply choke point (getRowPolicyFilter),
which every read path goes through (both engines, StorageMerge, view
inlining, and persisted policies).
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Replace the blunt is_internal_backup exemption with propagating the initiator's s3_allow_server_credentials_in_user_queries value through the DDL constraint clamp, scoped to BACKUP/RESTORE queries only. The value already rides in the DDL entry; the clamp was dropping it on hosts whose default profile pins the setting readonly. A restricted initiator's readonly constraint keeps the entry value at 0, so its per-host operations (including lazy base_backup opens) stay fully restricted, while backup-internal's value of 1 is honored. Every other DDL keeps the strict clamp.
Adds a regression test proving an untrusted initiator cannot reach server credentials on other hosts via a bare-URL base_backup.
Co-Authored-By: Claude Fable 5 <[email protected]>
The FWCC sparse-preservation change added an always-shrink branch when
`filter.isSparse()`, dropping the dense-path `2 * zeros > size` floor on
the grounds that per-granule tail counting is cheap for sparse filters.
Cheap to compute isn't cheap to apply: each shrunk tail ends the delayed
read and forces a fresh seek for the next granule. On remote storage
with a highly sparse column this fragments the read into ~2x as many
non-adjacent ranges and misses the filesystem cache segments the earlier
run cached, adding ~8x seeks. ClickBench Q10/Q11 lost 15-30% on master
26.7.1.997 (aws hot median).
Use the same profitability floor for sparse and dense filters. Local
repro (prod-1, 100M hits): Q11 hot median 256ms -> 100ms, SelectedRanges
20 -> 9.
Reported at https://github.com/ClickHouse/clickhouse-private/issues/63870
Co-Authored-By: Claude Opus 4.7 <[email protected]>
BACKUP/RESTORE ON CLUSTER ships the initiator's settings in the DDL entry, but each host clamps them against the default profile (no user is set for distributed DDL by default), silently dropping s3_allow_server_credentials_in_user_queries=1 when the default profile pins it with a readonly constraint. The internal per-host queries now defer the restriction to the initiator, which already opened the same backup destination under its own user settings.
Co-Authored-By: Claude Fable 5 <[email protected]>
The check lived in the shared updateRowPolicyFromQueryImpl, which is also
used by AccessEntityIO::deserializeAccessEntity. That made loading a
persisted policy containing arrayJoin fail while building the row policy
cache, breaking sessions instead of failing the query cleanly.
Move the create-time check to InterpreterCreateRowPolicyQuery::execute so
it only rejects user-facing CREATE/ALTER. Persisted, replicated and
restored policies load unchanged and are rejected by the query-time guard
in ContextAccess::getRowPolicyFilter when they are actually used.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The struct-into-Nullable gate only checked `allow_experimental_nullable_tuple_type`,
but the library reader also enables `Nullable(Tuple(...))` when the requested target
type is already nullable. So reading Arrow data with the setting off into an existing
`Nullable(Tuple)` column (or nested `Array(Nullable(Tuple))`) dropped the struct-level
null map and decoded the child payload as a real value row instead of `NULL`.
`decodeField` now wraps a struct in Nullable when the setting is on OR the effective
target hint at this field is nullable (matching the library's `allow_nullable_struct`),
resolving the hint via `resolveTargetHint` so both top-level and nested fields honor it.
`decodeColumns` reconciles the reported type with the decoded column via
`matchColumnNullability`, so the column/type pair stays consistent when `decodeField`
wraps a struct that `fieldToCHType` (setting-only) left as a plain Tuple.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
A row policy USING expression is applied as a per-row filter at the
storage read stage (a row-level, PREWHERE-like step). arrayJoin changes
the number of rows, which breaks the reader's invariant and aborts with
the logical error 'column->size() == num_rows'.
Reject arrayJoin in a row policy filter both when the policy is created
or altered and when it is applied, so existing on-disk or config-defined
policies fail the query cleanly instead of crashing. arrayJoin scoped
inside a nested subquery is still allowed, as it does not change the
outer row count.
CI report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=109493&sha=6774b64cd3bd77f1c9b010978b9810caa9dd015d&name_0=PR&name_1=Stress%20test%20%28amd_tsan%29
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
filterPartsByStatistics prunes parts from per-column min/max estimates of the
on-disk (pre-mask) values, so `WHERE masked_col = x` could prune parts that
masking would turn into matches. Disable it when a masking policy applies, the
same way it is already disabled for on-the-fly mutations and patch parts.
