native: analyse the tic statements with optimize_and_compare_chain = 0 - #598
Merged
Merged
Conversation
With each comparison in an AND or OR chain wrapped in identity(), the first tic statement still spends part of its analysis in LogicalExpressionOptimizerVisitor::tryOptimizeAndCompareChain, which infers comparisons from chains such as x = y AND y = 5. The setting optimize_and_compare_chain gates it. Turning it off takes the bench stage1 analysis from a median of 1.14 s to 0.94 s on 26.8.2.7 (five interleaved runs each), paid once per resident session and once per statement pair a test issues. The resident sessions send it through resident_settings, and the statements run_statement, demo_statement and the bench cuts build carry it as a URL setting, both from one ANALYSIS_SETTINGS constant, so a test's analysis matches a session's. Statement::with adds to the settings a statement already has instead of replacing them, so a tic statement carries both its parse settings and the analysis settings. NATIVE.md lists the setting beside the others a resident sends. The rows are unchanged: a 300-tic resident walk with and without the setting writes identical native_state and native_stage cells, and native diff 274 still agrees with the probe trace over 273 tics. Closes #581.
MarcusKainth
marked this pull request as ready for review
September 25, 2026 14:58
1 task done
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.
What this changes, and why
Closes #581.
The resident sessions and the tic statements a test issues both send
optimize_and_compare_chain = 0. The value lives in one place,ANALYSIS_SETTINGSinnative/src/resident/settings.rs.resident_settingsappends it, andstage1_overandrun_statementintick.rsattach it withStatement::with. That coversrun_statement,demo_statementand thebenchcuts, because all of them build their first statement throughstage1_over.Statement::withappends to a statement's settings instead of replacing them, so a tic statement carries its parse settings and the analysis settings together. Its only callers are intick.rs.NATIVE.mdtakes the contract text from the issue as written.Evidence
All live runs are against a throwaway
clickhouse/clickhouse-server:26.8.2.7container on port 18140, on an Apple M5 Max. The "without" binary is this commit with the fivesettings.extend(ANALYSIS_SETTINGS...)lines removed fromresident_settings, rebuilt and then restored withgit checkout HEAD --. That means the only difference between the two resident sessions is the setting. The statement text is the same in both.The unit test fails when either path loses the setting
a_test_s_statements_are_analysed_as_a_session_s_areintick.rs. The test was committed first, then one path was broken at a time, the test was run, and the file was restored withgit checkout HEAD --.The
.with(&ANALYSIS_SETTINGS)removed from the second statement inrun_statement:The
settings.extend(...)removed fromresident_settings:A 300-tic resident walk writes the same rows with and without the setting
The probe trace comes from
make gen-probe-traceat this commit (# wrote 2172 rows over 2172 frame commits, exit 0).native load --freshandnative load --probeboth exited 0. Each arm then rannative diff 300 --probe <trace> --record <file>. Each run exits 3 at the refused tic 275, but only after it has fed and written all 300 tics. After each run,native_stateandnative_stagewere copied into MergeTree snapshots.The comparison takes each
(tic, column)of the two snapshots, joined ontic, and comparescityHash64of the two cells. The column list is built fromsystem.columns: 148 columns fornative_stateand 151 fornative_stage, which match the live tables. The positive control copies the "without" snapshot and adds 1 toleveltimeat tic 150.(The output is
FORMAT Vertical, folded here onto one line per comparison.)Parity against the probe trace
This is the committed binary against the same full trace:
The "without" walk above refuses at 275 with the same
unresolved: CHASE_STUCKbits.The setting appears in
system.query_logfor both resident query idsThese are the
QueryFinishrows of the two walks. Pid 1034 is the "with" binary and pid 1608 is the "without" binary.The
--recordlines give the same analysis times:stage1_analysis_s0.955 andstage2_analysis_s0.157 with the setting, 1.198 and 0.351 without. These are single runs taken outside the machine lock.What stage1's analysis costs with and without the setting
The machine lock was taken with
scripts/machine-lock.sh acquire --force andchain "owner approved running under background load on 18 cores"(exit 0). The runs use a throwaway live test, not committed. It walks the level to tic 19 through a resident session. It then issuestick::bench::stage1(db, None, &[Input::demo(20)])ten times, interleaved ABBA. The "with" arm is the statement as built here. The "without" arm is the sameStatementwithoptimize_and_compare_chainremoved from its settings. Each figure isProfileEvents['QueryAnalysisMicroseconds']fromsystem.query_log, together with theSettingsmap value the server logged and the 1-minute load average before and after the run. No run was above 8, so none was discarded.optimize_and_compare_chain = 0The setting saves about 0.2 s per analysis. The issue measured about 0.3 s from a higher baseline of 1.32 s, at a load of 3.3 to 7.8.
Unit tests and lint
These were run on the committed tree after the scratch test was deleted:
Invariants
None. The change adds one server setting, which affects how a statement is analysed. No computation moves out of ClickHouse, and the statement text is unchanged. The walk comparison above shows every cell is the same with and without the setting.
Spec impact
NATIVE.mdchanges are included, and thespec-changeissue spec: send optimize_and_compare_chain = 0 with the resident statements #581 (ratified) tracks the decision.SPEC.mdis not touched.Checks
make gates: not run. The checks run instead weremake lint, the unit tests ofclickdoom-nativeandclickdoom-driver, and the live runs above. The live suites were not run.make native-smoke: not run.native diff 274against the full probe trace exercises the simulation. The renderer's statement also runs underresident_settings, and no render run was made.Anything else
The render resident also takes
resident_settings, so it now runs with the setting too. No frame comparison was made with and without the setting.Written mostly by Claude Opus 5.5.