Skip to content

Fix crash in getTopKMarks when limit is zero#95072

Merged
alexey-milovidov merged 1 commit intomasterfrom
fix-top-k-limit-zero
Jan 25, 2026
Merged

Fix crash in getTopKMarks when limit is zero#95072
alexey-milovidov merged 1 commit intomasterfrom
fix-top-k-limit-zero

Conversation

@alexey-milovidov
Copy link
Copy Markdown
Member

@alexey-milovidov alexey-milovidov commented Jan 25, 2026

When LIMIT -0. (or any expression that evaluates to 0) is used with use_skip_indexes_for_top_k = 1, the getTopKMarks functions would call queue.top() on an empty priority queue, causing undefined behavior and a segmentation fault.

The fix adds an early return when n == 0 in both getTopKMarks overloads, since requesting the top 0 elements should return nothing.

Changelog category (leave one):

  • Critical Bug Fix (crash, data loss, RBAC) or LOGICAL_ERROR

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fix crash in the top K optimization when LIMIT is zero. Closes #93893.

See https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=95065&sha=e047a3f739277d9272ec5108eb879df601c7aaf7&name_0=PR&name_1=AST%20fuzzer%20%28arm_asan%29

Checked manually that the new test reproduces the issue on the debug build.

When `LIMIT -0.` (or any expression that evaluates to 0) is used with
`use_skip_indexes_for_top_k = 1`, the `getTopKMarks` functions would
call `queue.top()` on an empty priority queue, causing undefined
behavior and a segmentation fault.

The fix adds an early return when `n == 0` in both `getTopKMarks`
overloads, since requesting the top 0 elements should return nothing.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Jan 25, 2026

Workflow [PR], commit [8273bfb]

Summary:

job_name test_name status info comment
BuzzHouse (amd_debug) failure
Logical error: 'Inconsistent AST formatting: the query: (STID: 1941-1bfa) FAIL cidb, issue
Finish Workflow failure
python3 ./ci/jobs/scripts/workflow_hooks/pr_body_check.py failure

@clickhouse-gh clickhouse-gh bot added pr-critical-bugfix pr-must-backport Pull request should be backported intentionally. Use this label with great care! labels Jan 25, 2026
@shankar-iyer
Copy link
Copy Markdown
Member

Thanks for the fix! I will adjust my code change for same issue in #93893

@shankar-iyer shankar-iyer added v25.12-must-backport and removed pr-must-backport Pull request should be backported intentionally. Use this label with great care! labels Jan 25, 2026
@alexey-milovidov alexey-milovidov merged commit 2b25eba into master Jan 25, 2026
130 of 134 checks passed
@alexey-milovidov alexey-milovidov deleted the fix-top-k-limit-zero branch January 25, 2026 16:32
@robot-clickhouse-ci-1 robot-clickhouse-ci-1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Jan 25, 2026
@robot-ch-test-poll1 robot-ch-test-poll1 added the pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR label Jan 25, 2026
robot-clickhouse-ci-2 added a commit that referenced this pull request Jan 25, 2026
Cherry pick #95072 to 25.12: Fix crash in `getTopKMarks` when limit is zero
@robot-clickhouse robot-clickhouse added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Jan 25, 2026
clickhouse-gh bot added a commit that referenced this pull request Jan 25, 2026
Backport #95072 to 25.12: Fix crash in `getTopKMarks` when limit is zero
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-critical-bugfix pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR pr-synced-to-cloud The PR is synced to the cloud repo v25.12-must-backport

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Segmentation fault (STID: 1726-5e68)

5 participants