{{ message }}
Explain pretty make default#105036
Merged
Merged
Conversation
Contributor
clickhouse-gh
Bot
added
the
pr-improvement
![clickhouse-gh[bot]](https://avatars.githubusercontent.com/in/928874?s=60&v=4){kind=small}
Adds the missing entry to SettingsChangesHistory's 26.5 block so that `SET compatibility = '25.5'` (or any version older than 26.5) reverts `query_plan_pretty_default` to its pre-26.5 value `false` via `applyCompatibilitySetting`. Without this entry, the test `04219_explain_plan_pretty_default` (test 1 of 5) fails — it asserts that the `compatibility = '25.5'` knob rolls the new pretty default back. `02995_new_settings_history` was also failing because it cross-checks that newly declared settings have a corresponding entry in the history. CI report: #105036 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A second batch of stateless tests that the previous opt-out commits
missed. All of these inspect EXPLAIN PLAN / EXPLAIN AST output (either
matching it against a reference file, grepping it for substrings, or
counting lines), and so they break when the default flips to the
compact/pretty/action-annotated form.
The fixes are:
- 14 .sql tests without `-- { echo }`: prepend
`SET query_plan_pretty_default = 0;` (placed after any leading Tags
block, so `getTestTagsLength` still strips it).
- 3 .sh tests (`02303_query_kind`, `04093_lazy_final_combined_filtering`,
`04095_lazy_final_with_lazy_materialization`): append
`--query_plan_pretty_default=0` to `$CLICKHOUSE_CLIENT`
(and `$CLICKHOUSE_LOCAL` in `02303_query_kind`, which also calls
`clickhouse-local`).
- `03280_aliases_for_selects_and_views`: move the SET line below the
two `EXPLAIN AST ... -- { clientError SYNTAX_ERROR }` queries.
Placing it above breaks `TestHint` detection of the expected client
error, the same regression that commit fdcb993 on master already
fixed for `SET enable_analyzer`.
Known still-failing: tests with `-- { echo }` / `-- { echoOn }` where
the SET on line 1 displaces `-- Tags:` from the first line — so
`getTestTagsLength` no longer strips the Tags block and those lines
end up echoed before the rest of the test. Fixing those needs the
reference files to be regenerated locally, so they're left for a
follow-up commit.
CI report: #105036
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
rienath
requested changes
May 15, 2026
rienath
left a comment
rienath
left a comment
Member
There was a problem hiding this comment.
Overall looks good. I've left some comments. Let's also mention the new query_plan_pretty_default setting in the changelog
Contributor
|
📊 Cloud Performance Report ✅ AI verdict: This PR only makes the pretty/compact/actions form the default for EXPLAIN PLAN output (settings, interpreter formatting, docs, and test pins) and leaves the query-execution hot path untouched. The two flagged ClickBench improvements (Q4 -17.4%, Q15 -16.8%) are plain SELECTs that never exercise EXPLAIN, so the PR cannot plausibly speed them up; both deltas are consistent-looking run-to-run variance and have been downgraded to not-sure. No real performance change is expected from this change. clickbenchFlagged queries (2 of 43)
q-value = BH-FDR adjusted p; smaller is stronger evidence. MIRAI flags a query when q < fdr_q (default 0.10) — the value the verdict is based on. tpch_adapted_1_official🟢 No significant changes Debug info
|
…n-pretty-make-default
…o explain-pretty-make-default
…lickHouse into explain-pretty-make-default
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
…n-pretty-make-default
…n-pretty-make-default
Contributor
LLVM Coverage Report
Changed lines: Changed C/C++ lines covered by tests: 46/47 (97.87%) | Lost baseline coverage: none · Uncovered code |
Contributor
Author
alexey-milovidov
added a commit
to zex-hyd/ClickHouse
that referenced
this pull request
Jul 3, 2026
…lan_default` default flip This test failed on CI (result differs with reference) for two independent reasons on the current head: 1. The `.sql` printed the label `-- Additional WHERE clauses present, 2 full parts selected by partition key / 1 part partially selected by PK` while the `.reference` expected the same line suffixed with `, expect index usage`. The query below it now uses the vector index under partial primary key pruning (the point of this PR), so `USearchSearchCount` is `1` and the label should read `expect index usage`. Align the `.sql` literal with the `.reference`. 2. `ClickHouse#105036` made `explain_query_plan_default = 'pretty'` the default on master, which renders `EXPLAIN` plans with box-drawing characters that `trimLeft` does not strip, so the grepped `Description:`/`Granules:` lines no longer match the plain reference. Master added `SET explain_query_plan_default = 'legacy';` to this test for exactly this reason; do the same here. Verified end-to-end against the PR build (26.7.1.1): the test now matches its reference. CI report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=104754&sha=9d1b3aec2a86952d71263172400a8a97bcb05c0f&name_0=PR Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
alexey-milovidov
added a commit
that referenced
this pull request
Jul 3, 2026
The recent master merge brought in `explain_query_plan_default = 'pretty'` (the new default since 26.7, introduced in #105036), which changes the default `EXPLAIN PLAN` output format. The five constant-folding tests that assert on `EXPLAIN indexes = 1` output started failing in Fast test with "result differs with reference": `04218_partition_pk_pruning_constants_folding`, `04219_partition_minmax_skip_index_constants_folding`, `04220_partition_id_constants_folding`, `04221_partition_id_with_minmax_constants_folding`, `04314_partition_constants_folding_derived_key`. Add `SET explain_query_plan_default = 'legacy';` to each of them so their existing (already reviewed) references stay valid. This mirrors how #105036 handled the ~414 existing `EXPLAIN` tests when it changed the default, and keeps the references demonstrating per-part constant folding unchanged. CI report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=104582&sha=ebf528aa38a90f8bc69735ead2da02540b2d479b&name_0=PR&name_1=Fast%20test Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
UnamedRus
added a commit
to UnamedRus/ClickHouse
that referenced
this pull request
Jul 3, 2026
`EXPLAIN` output became pretty by default on master (ClickHouse#105036, `explain-pretty-make-default`). In pretty mode `JoinStep` prints the join clauses under a `Join conditions` node instead of the flat `Clauses:` line, so the test's `WHERE explain ILIKE '%Clauses%'` filter matched nothing and the reference no longer applied. That master change updated the references of existing tests, but could not touch 04326 because the test only exists on this branch. Add an explicit `pretty = 0` to every `EXPLAIN actions = 1` so the flat `Clauses:` format (and the committed reference) stay valid regardless of the server default. Related: ClickHouse#106955 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
5 participants
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.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Make
EXPLAIN [PLAN] actions=1, compact=1, pretty=1the default.Version info
26.7.1.194