Add a test for pipeline suck with sort overflow and window functions#98543
Merged
alexey-milovidov merged 7 commits intomasterfrom Mar 5, 2026
Merged
Add a test for pipeline suck with sort overflow and window functions#98543alexey-milovidov merged 7 commits intomasterfrom
alexey-milovidov merged 7 commits intomasterfrom
Conversation
#57728 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
clickhouse-gh
bot
added
the
pr-not-for-changelog
…erflow_mode = 'break'` The test only needs to verify the query completes without getting stuck, not check the exact output value. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
alexey-milovidov
changed the title
…ctions Window functions require fully sorted input data. When `sort_overflow_mode = 'break'` was applied to the sorting step used by window functions, the `LimitsCheckingTransform` would call `stopReading()` early, leaving `ScatterByPartitionTransform` with data queued for a finished output port, causing a pipeline deadlock. The fix overrides `sort_overflow_mode` to `throw` for window function sorting in both the new planner and the old `InterpreterSelectQuery` paths. This ensures window function sorting always completes or throws a clear error, preventing the pipeline from getting stuck. Closes #57728 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
clickhouse-gh
bot
added
pr-bugfix
alexey-milovidov
commented
Mar 5, 2026
Member
Author
alexey-milovidov
left a comment
alexey-milovidov
left a comment
There was a problem hiding this comment.
This is trivial and good.
zlareb1
added a commit
to zlareb1/ClickHouse
that referenced
this pull request
Mar 6, 2026
…ctions Covers both the new analyzer path (Planner.cpp) and the old planner path (InterpreterSelectQuery::executeWindow) for the fix in ClickHouse#98543. Window functions require fully sorted input. When sort_overflow_mode = 'break' is set, LimitsCheckingTransform stops reading early, leaving ScatterByPartitionTransform with data queued for a finished output port, causing a pipeline deadlock. The fix overrides sort_overflow_mode to 'throw' for window function sorting in both planner paths. The PR's original test covered only the new analyzer path (enable_analyzer = 1 by default). This test adds the enable_analyzer = 0 variant to ensure InterpreterSelectQuery::executeWindow is also covered. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
zlareb1
added a commit
to zlareb1/ClickHouse
that referenced
this pull request
Mar 6, 2026
…s_to_sort test PR ClickHouse#98543 fixed a pipeline deadlock when sort_overflow_mode = 'break' is used with window functions by overriding it to 'throw' in both Planner.cpp (new analyzer) and InterpreterSelectQuery.cpp (old planner). Its test only covered the new analyzer path. Adds both paths to the existing 01131_max_rows_to_sort test which already covers sort_overflow_mode behaviour. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 task
zlareb1
added a commit
to zlareb1/ClickHouse
that referenced
this pull request
Mar 6, 2026
…ctions Covers both the new analyzer path (Planner.cpp) and the old planner path (InterpreterSelectQuery::executeWindow) for the fix in ClickHouse#98543. Window functions require fully sorted input. When sort_overflow_mode = 'break' is set, LimitsCheckingTransform stops reading early, leaving ScatterByPartitionTransform with data queued for a finished output port, causing a pipeline deadlock. The fix overrides sort_overflow_mode to 'throw' for window function sorting in both planner paths. The PR's original test covered only the new analyzer path (enable_analyzer = 1 by default). This test adds the enable_analyzer = 0 variant to ensure InterpreterSelectQuery::executeWindow is also covered. Co-Authored-By: Claude Sonnet 4.6 <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
3 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.
Summary
sort_overflow_mode = 'break'is used with window functionssort_overflow_mode = 'break'was applied to the sorting step used by window functions,LimitsCheckingTransformwould callstopReading()early, leavingScatterByPartitionTransformwith data queued for a finished output port — causing a pipeline deadlocksort_overflow_modetothrowfor window function sorting in both the new planner (Planner.cpp) and the oldInterpreterSelectQuerypathsTOO_MANY_ROWS_OR_BYTESinstead of getting stuckCloses #57728
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
sort_overflow_mode = 'break'together with window functions.