feat(clickhouse): P0 + P1 of the 4-month coverage-gap audit (items 0a, 0b, 1-18) - #11
Open
fm4v wants to merge 4 commits into
Open
feat(clickhouse): P0 + P1 of the 4-month coverage-gap audit (items 0a, 0b, 1-18)#11fm4v wants to merge 4 commits into
fm4v wants to merge 4 commits into
Conversation
…ems 0a, 0b) Both families cost triage time on every NightlySQLancer run and both are violations of the fork's own soundness checklist. 0a. LimitRanking's LIMIT-BY cap was measured client-side: it read the key column through ComparatorHelper.getResultSetFirstColumnAsString, which routes every value through trimTrailingDotZeros. That helper rewrites '0.0' into '0', so a String key holding both values looked like one key appearing twice and the oracle reported a cap violation that did not exist (2026-08-04 and 2026-08-07 reproducers; replay against head shows uniqExact(c0) = count() = 10000 and LIMIT 1 BY returning exactly 10000 rows). The cap is now computed in ClickHouse: SELECT max(cnt) FROM (SELECT count() AS cnt FROM (<limit-by query>) GROUP BY lb_key) which is also cheaper, since 10000 rows no longer cross the wire. Verified on head 26.8.1.1470 against the exact false-positive shape: a String key holding '0.0', '0', '0.0' and 'x' reports max-per-key 1 under LIMIT 1 BY and 2 under LIMIT 2 BY, so the assertion still measures the real cap. Note that trimTrailingDotZeros is still applied by every other oracle. Scoping it to float columns, or replacing it with the ULP-tolerant comparison mode that already exists in ComparatorHelper, is a separate follow-up. 0b. The engine pool could pick ReplacingMergeTree() with a Bool sorting key and no ver argument. With a two-value key a background merge collapses visible cardinality between two reads, which produced the 2026-08-07 TLPWhere "size of the result sets mismatch (91 and 26)" report (91 = 7x13 before the merge, 26 = 2x13 after). Dedupe and collapse engines now require a non-degenerate key domain via hasDegenerateKeyDomain / isDedupeKeyColumn: Bool is rejected, and so is an Enum with fewer than MIN_DEDUPE_KEY_DOMAIN (8) entries, which today means every generated Enum because the type picker caps them at 5. pickEngine falls back to plain MergeTree when no eligible column exists, the dedupe fallback ORDER BY uses the same filter, and ReplacingMergeTree now always emits its ver argument instead of doing so half the time. Verified mid-run against system.tables on a 12-minute dev-VM run: 0 dedupe tables with a Bool or Enum sorting key, and 0 of 75 ReplacingMergeTree tables without a ver argument.
Six coverage items from docs/plans/2026-08-15-001-feat-clickhouse-4month-
coverage-gap-plan.md. Each new oracle is gated by a default-on ClickHouseOptions
flag, has one ClickHouseOracleFactory entry, and one ALL_ORACLES token.
Item 1, boolean-position and truth-value predicates
(--truth-value-predicate-emission). generatePredicate() gains two arms: numeric
columns wrapped in NOT (NOT x), NOT x, x IS [NOT] TRUE/FALSE/UNKNOWN,
x IS NOT DISTINCT FROM lit, nullIf/ifNull/coalesce(x, lit), and String columns
under LIKE/ILIKE ... ESCAPE. Half the time the wrapper is compared against a
numeric or float constant, which puts a boolean-valued expression in *value*
position -- the shape KeyCondition's inversion pushdown mishandles. Rendered
through real AST nodes (ClickHouseUnaryPrefixOperation, ClickHousePostfixText,
and the new ClickHouseWrappedExpression) rather than ClickHouseRawText, so the
KeyCondition oracle's materialize() rewrite still reaches the column references.
This immediately surfaces a real, unfiled wrong result on head 26.8.1.1470:
(NOT (NOT c1)) <= 3.14 evaluates to 1 for every row in a projection, but as a
WHERE clause it prunes parts, and EXPLAIN indexes=1 prints
"Condition: (c1 in (-Inf, 3])". Root cause is the name == "not" branch of
cloneDAGWithInversionPushDown in src/Storages/MergeTree/KeyCondition.cpp
ignoring boolean_context, so two flips cancel and NOT NOT c1 degrades to bare
c1. Wrong since at least 24.8. NoREC and TLPWhere catch it; KeyCondition does
NOT, because neither materialize() nor any settings profile disables this
pruning path.
Item 2, ClickHouseFloatPruningOracle (--float-pruning-oracle). Private fixture
with Float32/Float64/Nullable(Float64) columns holding NaN, +/-inf, -0.0 and
NULL across several parts (one part all-NaN), with a float ORDER BY, an optional
float PARTITION BY, minmax and bloom_filter skip indexes and materialized
statistics. Two assertions: a negated float comparison in WHERE must select the
same key multiset as the same predicate evaluated as a groupArrayIf aggregate
argument over a full scan, and count(P) + count(NOT P) + count(P IS NULL) must
equal count(*).
The full-scan reference is load bearing. The plan specified materialize() plus a
pruning-off settings profile, and that is not enough: verified on head, none of
materialize(), use_skip_indexes=0, use_skip_indexes_on_data_read=0,
allow_statistics_optimize=0, convert_query_to_cnf=0, optimize_move_to_prewhere=0,
force_primary_key=0 or query_plan_enable_optimizations=0 defeats partition-level
or primary-key-level pruning, so that arm would have compared two equally-wrong
answers. A predicate that never reaches a WHERE clause cannot be pruned; copy
that pattern for any future pruning oracle.
No float aggregate is computed anywhere, only count() and a key-column row set,
so the exact-integer-aggregate rule is not violated.
Item 3, ClickHouseDistributedPlanEquivalenceOracle
(--distributed-plan-equivalence-oracle). One generated read must return the same
multiset under plain local execution, make_distributed_plan = 1,
serialize_query_plan = 1, a cluster('default', ...) read with
parallel_replicas_local_plan on and off, and enable_parallel_replicas = 1 with
max_parallel_replicas = 3 over both the local relation and a Distributed(...)
wrapper. Five query shapes, one of which is a three-way comma join whose middle
relation is a VIEW (the ClickHouse#111727 shape). Self-contained multi-block
fixture. The single-node default cluster exists on head, so all six profiles
genuinely execute rather than silently erroring out.
Item 4, views and comma joins in multi-relation FROM lists
(--persistent-view-emission, --comma-join-emission). Two independent gaps:
- Views were already visible to the join picker, but ViewEquivalence creates
and drops its view inside a single iteration, so no schema snapshot ever
held one. A VIEW provider action now creates up to 3 plain v<n> views per
database (the schema reader marks a relation as a view by its name prefix).
- Every CROSS join was silently an INNER join, because the generator always
handed it an ON clause. FROM t0, v0, t1 was therefore unreachable. The join
generator now emits genuine ON-less CROSS joins and chains of up to four
relations, and the visitor renders an ON-less CROSS as a comma. A bare
"JOIN x" with no ON is a SYNTAX_ERROR in ClickHouse, hence the comma.
Comma joins are rate-limited to 10% of CROSS picks on purpose. At 50% a
40-minute dev-VM run spent roughly 40% of its thread budget on three- and
four-way cartesian products timing out at max_execution_time, and throughput
fell from about 100 to about 10 queries/s. At 10% throughput holds at 75-95
queries/s and the shape still appears about 90 times per 30 minutes.
Because views are now visible to every oracle, write paths must filter them:
ClickHouseAlterGenerator and ClickHouseMutationGenerator move to
getDatabaseTablesWithoutViews(), ClickHouseCERTOracle, ClickHouseRowPolicyOracle
and ClickHouseQueryConditionCacheOracle grow !isView() filters, TLPBase gates
PREWHERE on !isView(), and ILLEGAL_PREWHERE plus "is not supported by storage
View" are tolerated globally as a backstop. Any new oracle that INSERTs, ALTERs
or OPTIMIZEs a schema-picked table must do the same.
ClickHouse#114113 ("Left and right columns have same names" out of
chooseJoinOrder, a server abort on sanitizer builds) is pinned via
ClickHouseErrors.getKnownOpenJoinOrderBugs() so runs do not drown in it. It did
not reproduce on the release build 26.8.1.1470 with the plan's minimal repro.
Remove the pin when the issue closes.
Item 5, join-order enumerator sweep. ClickHouseJoinReorderOracle runs the same
N-way join under query_plan_optimize_join_order_algorithm in {greedy, dpsize,
dpsub, dphyp, dphyp+greedy, dpsub+greedy}, plus
query_plan_enable_optimizations = 0, query_plan_join_shard_by_pk_ranges = 1 and
query_plan_optimize_join_order_max_searched_plans = 1, and asserts identical
multisets against the default-arm result. The oracle also builds a VIEW over one
of its private tables 40% of the time, which is the deterministic delivery
vehicle for the view-in-multi-join shape.
The setting is query_plan_optimize_join_order_algorithm, not
query_plan_join_reorder_algorithm as the plan guessed. dpsize and dphyp support
inner joins only and raise Code 717 EXPERIMENTAL_FEATURE_ERROR "Failed to find a
valid join order, try adding 'greedy' algorithm as fallback" on outer, semi and
anti chains; that is a legitimate unsupported-shape error and is tolerated in a
dedicated algorithmErrors set. Without that tolerance the first validation run
produced 924 junk reproducers in 12 minutes.
Item 6, ClickHouseCodecRoundtripOracle (--codec-roundtrip-oracle). A table with
random per-type CODEC(...) declarations and a CODEC(NONE) mirror holding the
same inserted rows (including NaN, +/-inf, -0.0 and denormals) must answer the
same read identically, still after OPTIMIZE ... FINAL, and still after an
ALTER TABLE ... MODIFY COLUMN ... CODEC mutation, which is where mixed-codec
parts and adaptive selection come in. The coded table sometimes carries
allow_experimental_adaptive_codec_selection = 1. Lossy codecs (SZ3, ZXC) are
partitioned off the equality arm by an explicit allowlist and only have their
row count and NULL mask asserted; if the lossy DDL is rejected the oracle
retries with a lossless float codec rather than dropping the iteration. ALP was
also added to the general schema's float codec pool.
FloatPruning is deliberately absent from ALL_ORACLES. It is a positive-control
detector for ClickHouse#113417 and #112036, which reproduce on head at DEFAULT
settings, so it asserts on nearly every iteration: a 6-minute standalone run
produced 326 worker deaths over 175 queries. A constantly firing oracle orphans
a database per iteration and wedges the server under a squeezed memory cap,
which is what stalled the 2026-06-14 20h run via TextIndexDirectRead. Run it
with --oracles FloatPruning, and add it back here once those issues close.
…g plan assumptions
Adds a "P0 coverage batch, 2026-08-15" section to the provider CLAUDE.md with
per-item operational detail, and a "Known-open bugs the 2026-08-15 batch
deliberately fires on" triage list so a future run recognises the expected noise
instead of re-investigating it:
- the unfiled NOT (NOT key) part-pruning wrong result, with the reason
KeyCondition cannot catch it and NoREC/TLPWhere can;
- ClickHouse#113417 / #112036, why FloatPruning is kept out of ALL_ORACLES;
- ClickHouse#114113 and its pin.
Marks items 0a, 0b and 1-6 done in the plan, flips its status to
p0-implemented, and records where the plan was wrong so the P1/P2 entries are
not written against the same false premises:
- materialize() plus a pruning-off settings profile does not defeat
partition-level or primary-key-level pruning, which invalidated the
intended oracle for item 1 and the reference arm for item 2;
- the join-order enumerator setting is
query_plan_optimize_join_order_algorithm;
- item 4 needed a persistent-view DDL action and a real ON-less CROSS join,
not just a join-picker change.
Validation on dev-VM head 26.8.1.1470: a 30-minute full-fleet run over all 94
oracles except TextIndexDirectRead finished exit 0, 137,380 queries, 0
reproducers, 0 threads shut down. A 12-minute run of the changed and new
oracles did 36k queries with a single reproducer, and that one was the known
unfiled NOT (NOT bug.
Implements the whole P1 tier of docs/plans/2026-08-15-001-...-4month-coverage-gap-plan.md: three new oracles, arms on eight existing oracles, and four generator changes. New oracles: PipeEquivalence (pipe-operator syntax, PR #111151), IEJoin (two-inequality ON, PR #109920), TupleFinalAggregation (per-element Tuple aggregation in Summing and Coalescing engines, PR #98039). Generator work: GROUPS window frames plus explicit ROWS/RANGE frames, which the window generator previously never emitted at all; negative LIMIT / LIMIT BY / WITH TIES forms; LIKE-OR and !=-AND predicate chains so optimize_or_like_chain and optimize_and_compare_chain are reachable; uniq_v2 and basic statistics everywhere plus an ADD STATISTICS form and auto_statistics_types table settings; descending and mixed-direction sorting keys; sparse serialization that actually engages (low ratio_of_defaults_for_sparse_serialization plus default-biased inserts on plain MergeTree tables); the icu tokenizer. Oracle work: a GROUPS peer-group ground truth and the two degenerate GROUPS identities; negative-limit reversal identities; an indexHint containment arm; query-condition-cache ORDER BY LIMIT coverage, the two open poisoning shapes as triggers, and a re-verify-after-DROP step that separates a cache bug from a merge artifact; hasPhrase with a token-position ground truth, a trivial-count-from-text-index arm and text index parameters as table settings; parallel_full_sorting_merge in the join-algorithm sweep; multi-key GROUP BY and mixed-direction ORDER BY in the read-in-order sweep. Five plan assumptions were wrong and are corrected in the code and both documents: indexHint is not result-neutral (it restricts the read to the granules index analysis selects, so the invariant is containment, and it must not be emitted into the general fleet where TLP branches would read different granule sets); ASC-to-DESC is not an order reversal because ClickHouse sorts NULLs last in both directions; each pipe stage is wrapped in a subquery, so qualified names and MATERIALIZED columns do not survive it; ie_join is the only algorithm that can answer a two-inequality ON, so the reference arm is a CROSS JOIN; null_count statistics do not exist on head. Validated on dev-vm head 26.8.1.1473: 30 minutes over all 97 oracles except TextIndexDirectRead, 162,814 queries, 0 reproducers, 0 threads shut down, with every new arm confirmed present in system.query_log. Two wrong results found on the way, both unfiled and documented in .claude/CLAUDE.md: optimize_aggregation_in_order collapses every GROUP BY group over a DESC sorting key, and an integer constant inside indexHint is narrowed to UInt8 so any multiple of 256 prunes every granule. Also fixes three test expectations left stale by the P0 batch's comma-join rendering.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the P0 and P1 tiers of
docs/plans/2026-08-15-001-feat-clickhouse-4month-coverage-gap-plan.md, the coverage-gap audit of everything ClickHouse merged between 2026-04-15 and 2026-08-15. P0 (items 0a, 0b, 1-6) landed in the first three commits and was validated on head 26.8.1.1470; the fourth commit adds all of P1 (items 7-18).Why
The audit was grounded in a measured miss: on 2026-08-13 a 10-minute run of upstream SQLancer found a silent wrong result that this fork's 98 oracles had never surfaced, because the gap was in the predicate grammar rather than in the oracle set. Three structural patterns explain the rest of the gaps: sound oracles starved by a narrow grammar, deliberate float avoidance that has become a blind spot, and whole young subsystems (plan-based parallel replicas, the new join-order enumerators, pipe operators, IEJoin, GROUPS frames, the second text-index wave) that the fork could not generate at all.
What is in P1 (items 7-18)
Three new oracles, all wired into
ALL_ORACLES:PipeEquivalenceIEJoinCROSS JOIN ... WHERETupleFinalAggregationTupleaggregation in SummingMergeTree and CoalescingMergeTree (PR #98039) == a Java element-wise ground truth, and query-timeFINAL== a physicalOPTIMIZE ... FINALGenerator work.
GROUPSwindow frames plus explicitROWS/RANGEframes (the window generator previously emitted no frame clause at all); negativeLIMIT/LIMIT BY/WITH TIES; longLIKE-OR and!=-AND predicate chains sooptimize_or_like_chainandoptimize_and_compare_chainare reachable at all;uniq_v2andbasicstatistics everywhere plus anADD STATISTICSform andauto_statistics_typestable settings; descending and mixed-direction sorting keys; sparse serialization that actually engages (lowratio_of_defaults_for_sparse_serializationplus default-biased inserts on plain-MergeTree tables, verified throughsystem.parts_columns); theicutokenizer.Oracle work. A GROUPS peer-group ground truth and the two degenerate GROUPS identities; negative-limit reversal identities; an
indexHintcontainment arm; query-condition-cacheORDER BY ... LIMITcoverage, the two open poisoning shapes as triggers, and a re-verify-after-DROPstep that separates a genuine cache bug from a merge artifact;hasPhrasewith a token-position ground truth, a trivial-count-from-text-index arm, and text index parameters supplied as table settings;parallel_full_sorting_mergein the join-algorithm sweep; multi-key GROUP BY and mixed-direction ORDER BY in the read-in-order sweep.Every new surface is behind a default-on flag (
--groups-window-frame-emission,--negative-limit-emission,--comparison-chain-emission,--index-hint-emission,--sparse-column-emission,--mixed-direction-sorting-key,--text-index-second-wave,--pipe-equivalence-oracle,--ie-join-oracle,--tuple-final-aggregation-oracle) plus one deliberately default-off arm,--summing-subset-projection-arm.Validation
Dev VM, fresh
clickhouse/clickhouse-server:head.TextIndexDirectRead, head 26.8.1.1473: 162,814 queries, 0 reproducers, 0 threads shut down.system.query_logfor that run, not just by the absence of failures: 7322 GROUPS frames, 8982 trivial-count-from-text-index reads, 4005hasPhrase, 2092ie_joinjoins, 1046 tuple-aggregation reads, 922 pipe queries, 685 negative LIMITs, 623uniq_v2statistics statements, 562 chain-rewrite flips, 414indexHintreads, 246icutokenizer statements, 90parallel_full_sorting_mergejoins, 6092 columns inSparseserialization.DROP STATISTICSversus in-flight-mutation DDL race (now tolerated) and theindexHintconstant-truncation bug below.Two unfiled ClickHouse wrong results found on the way
1.
optimize_aggregation_in_ordercollapses every GROUP BY group over a DESC sorting key. Exactly what item 18 was built to find. Needs a single part; type-independent. Same family as the open #111901, which is filed for the two-column case, but this single-column form is strictly smaller and is not on that issue.2. An integer constant inside
indexHintis narrowed to UInt8 during index analysis, so any multiple of 256 reads as false, the key condition becomes unsatisfiable and every granule is pruned. Same family as #112236.Five plan assumptions turned out to be wrong
Each was caught by a five-minute probe against a fresh head rather than by reading a PR description, and each changed the shipped design:
indexHintis not result-neutral. It does not evaluate its argument as a filter, but it does restrict the read to the granules index analysis selects, so rows outside them are legitimately dropped (5 rows versus 6 on head). The shipped invariant is containment,rows(P AND Q) ⊆ rows(indexHint(P) AND Q) ⊆ rows(Q), whose lower bound is the pruning-soundness assertion the item actually wanted. For the same reasonindexHintis deliberately not emitted into the general fleet: inside a TLP partition the three branches would read different granule sets and their union would no longer be the whole table.ASC NULLS LASTagainstDESC NULLS FIRST.SELECT *does not carry MATERIALIZED or ALIAS columns. Ignoring either rule produced 1184 Code-47 reproducers in one run.ie_join, and every other algorithm rejects a two-inequality ON outright, so the reference arm is the equivalent cross join.null_countstatistics do not exist on head (accepted:basic,countmin,minmax,tdigest,uniq,uniq_v2), and two item-13 sub-items are unreachable: the Japanese tokenizer needs a server-side dictionary, and no posting-list apply-mode setting exists.All of this is written up in the plan's new "P1 implementation status" section and in the
## P1 coverage batch, 2026-08-16section of.claude/CLAUDE.md, so a future run can triage against it.Still open
P2 (items 19-25) of the same plan.