feat(clickhouse): execute safe row-count materializations - #632
Merged
Merged
Conversation
zzylol
marked this pull request as ready for review
September 11, 2026 02:28
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
1 participant
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.
SQL row-count plans were rejected by a PromQL-only temporal-count guard, and the source binder could not represent their literal update. This change lowers safe row counts through the shared typed constant projection and the existing physical SUM accumulator, then combines their readout with ordinary ClickHouse exact DAG dependencies.
Before:
count(*)could not install a mixed SQL plan; relaxing that guard alone would also expose Planner's erased nullablecount(value)semantics. After: the Planner revision includes its reviewed nullable-count guard (#389),count(*)includes NULL-valued rows correctly, and unsupported nullable counts remain rejected. Named value sources must be non-null Float64 because the current ingest boundary cannot preserve arbitrary Int64 values. Producer Project subtrees are rejected until their computation is executed, preventingvalue * 2 AS valuefrom silently reading the original column.Verification: all 726 control-plane tests passed. The real ClickHouse/backend process test passed for SUM and count(*) mixed with an external exact branch, including a NULL row, population/time-boundary exclusions, and source-row deletion after materialization to prove state readout. Compiler counterexamples cover nullable count(value), unsupported Int64 source values and erased producer projections. A subsequent item=value/weight=1 regression also passed; keyed summary inputs are rejected until their item projection is supported, preventing the unit weight from replacing the actual item. The selected Count DAG and typed constant projection are recorded in
/mydata/clickhouse-o11y-main-results/automatic-count-planning.json.This PR does not claim count performance benefits or full SQL workload coverage. It stacks on #617 for existing exhaustive Planner/UnivMon compatibility and also includes the already-merged #625 shared projection contract; those dependency diffs disappear when #617 is replayed onto main. Independent bounded source review passed.