Fix test_explain_with_fetch on ClickHouse 26.7 - #145
Open
Maksim-Burtsev wants to merge 1 commit into
Open
Conversation
ClickHouse 26.7 made `explain_query_plan_default = 'pretty'` the default,
and the pretty plan carries a blank line between the output columns and
the plan tree. A blank TSV line decodes to an empty Record (that is what
the `WITH TOTALS` separator looks like), so indexing every row of an
EXPLAIN result stopped working.
EXPLAIN SYNTAX also renders operators as function calls now
("SELECT plus(1, 1)"), so the test no longer asserts the exact spelling.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Maksim-Burtsev
force-pushed
the
fix-explain-test
branch
from
August 21, 2026 20:38
43c06a1 to
8ef1975
Compare
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.
test_explain_with_fetchfails on master right now, and it has nothing to do with the library — CI just runs against a newer ClickHouse.The image is not pinned: the Makefile does
docker pull clickhouse/clickhouse-server, i.e. alwayslatest. Master looks green only because its last CI run was on June 8; the image has moved on since, and all of this happened after the last release. I pushed the very same commit (5fadd93) to my fork today and it fails there too, on this test and nothing else — 4 failed, 925 passed.What changed on the server side
1. The plan format. ClickHouse 26.7 made the pretty plan the default. The server describes its own setting as:
explain_query_plan_default—pretty(default since 26.7),legacy— pre-26.7 output (release notes, EXPLAIN docs, ClickHouse/ClickHouse#98500, RFC).The pretty plan has a blank line in it. A blank TSV line decodes to an empty
Record— which is deliberate, it is what theWITH TOTALSseparator looks like — sorow[0]on that row raisesIndexError: Empty row. May be it is result of 'WITH TOTALS' query.. The test indexes every row, so it breaks.2.
EXPLAIN SYNTAX. It now prints operators as function calls, soEXPLAIN SYNTAX SELECT 1 + 1returnsSELECT plus(1, 1)(plus aFROM system.oneline) instead ofSELECT 1 + 1.The fix
Keeps the intent of #98 — an EXPLAIN result comes back through
fetch/fetchvalas plan strings — and stops asserting the server's exact rendering: blank rows are skipped, and theEXPLAIN SYNTAXvalue is only checked to be the returned query.What I deliberately did not do:
EXPLAIN pretty=0 ...orSETTINGS explain_query_plan_default='legacy'): both the option and the setting only exist from 26.7, so that would break the test on older servers. I checked the new assertions against both output shapes on the same server, forcing the legacy format via the setting, and they hold for both.WITH TOTALSseparator are genuinely indistinguishable without a one-row lookahead in the streaming path. That is a behaviour change and deserves its own discussion, not a CI fix.Tests
Full suite, both builds (pure Python and with the Cython extension), against ClickHouse 26.7: 929 passed, 16 skipped, 0 failed.
One thing worth deciding separately: because the image is untagged, a server-side change like this lands in CI silently, on whatever PR happens to be open at the time. Pinning a major version in the Makefile would make CI reproducible, at the cost of noticing such changes later. Happy to do that in a follow-up if you want it.