Enable reading in reverse order with FINAL for ReplacingMergeTree - #111609
Draft
cwurm wants to merge 11 commits into
Draft
Enable reading in reverse order with FINAL for ReplacingMergeTree#111609cwurm wants to merge 11 commits into
cwurm wants to merge 11 commits into
Conversation
Second attempt at ClickHouse#58361, scoped to ReplacingMergeTree only. Addresses ClickHouse#58035. When a query with FINAL sorts in reverse order of the sorting key (e.g. ORDER BY key DESC LIMIT n), the read-in-order optimization now applies instead of falling back to a full read. ReplacingSortedAlgorithm learns a read_in_reverse mode: a row with a strictly higher version always replaces the selected one; among rows with equal (or absent) versions, the previously selected row is kept unless the current row comes from a newer data part. This mirrors the "last written row wins" rule of the direct reading order, because in the reverse reading order rows within one part arrive backwards while parts still arrive from the oldest to the newest one. The other engines keep the old behavior (no reverse reading with FINAL), since their merging algorithms rely on the direct order of rows: the sequence of sign rows in CollapsingMergeTree, the order of rows fed to order-dependent aggregate functions in AggregatingMergeTree, etc. The new setting optimize_read_in_reverse_order_final (default true) allows disabling the optimization. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Workflow [PR], commit [4b0c5c8] Summary: ❌
AI ReviewSummaryThis PR enables reverse-order read-in-order planning for Final VerdictStatus: ✅ Approve LLVM Coverage Report
Changed lines: Changed C/C++ lines covered: 174/179 (97.21%) · Uncovered code |
clickhouse-gh
Bot
added
the
pr-performance
![clickhouse-gh[bot]](https://avatars.githubusercontent.com/in/928874?s=60&v=4){kind=small}
…lacingMergeTree ReadFromMerge::requestReadingInOrder kept the old blanket guard against reverse order with FINAL, so a Merge table over ReplacingMergeTree children could not use the optimization. Replace the guard with a precise upfront check that every selected child table is a ReplacingMergeTree: the check must happen before delegating to the children, because the delegation loop switches the children to in-order reading one by one, and a child must not be left switched when a later child rejects the request. Addresses the review comment ClickHouse#111609 (comment). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…y block The 26.8 release cycle started on master, and the style check requires new settings to be recorded under the current version block. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A nested Merge table does not receive the read-in-order optimization in any direction, because recursivelyApplyToReadingSteps does not descend into the child plans of a nested ReadFromMerge (they are not plan-node children). The query falls back to the unoptimized plan and stays correct. Pin this behavior with a test. Related to the review comment ClickHouse#111609 (comment). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
LLVM Coverage Report
Changed lines: Changed C/C++ lines covered: 150/153 (98.04%) · Uncovered code |
topKThroughJoin chooses between itself and the second-pass read-in-order through the join, and blocked the deferral for every descending FINAL sort, because requestReadingInOrder used to reject a reverse direction with FINAL unconditionally. A query like `... FROM replacing FINAL LEFT JOIN ... ORDER BY key DESC LIMIT n` therefore kept the injected Sort + Limit over a full read instead of reading in reverse order. Extract the engine and setting check into ReadFromMergeTree::canReadInReverseOrderWithFinal so that both requestReadingInOrder and topKThroughJoin use one source of truth, and relax the guard accordingly. The conservative any_desc heuristic stays in place for the engines that do not support reading in reverse order. The test compares the rows selected by FINAL behind a join against a read with both optimizations disabled, for duplicates within a level-0 part, duplicates across parts, version ties, is_deleted, and descending sorting keys, so that a reverse read cannot silently pick another row of a duplicate key group. Addresses the review comment ClickHouse#111609 (comment). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The test pins that topKThroughJoin does not defer to a second pass that would reject the read, using a ReplacingMergeTree table sorted in descending order. Reading in reverse order with FINAL is now supported for that engine, so the second pass accepts it and the deferral is sound. Pin optimize_read_in_reverse_order_final = 0 to keep the test exercising a rejected read, which keeps its reference unchanged. The accepted case is covered by 04657_top_k_through_join_final_reverse_order. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
topKThroughJoin decided whether the second pass would reject a reverse FINAL read from the sort description alone (any column descending). That misses the sorting key's per-column reverse flags: a table ordered by `k DESC` queried as `ORDER BY k ASC` reads in reverse order even though no sort column is descending, so the deferral fired, the second pass rejected the read, and both optimizations were lost. It also blocked the deferral for the opposite case, a descending sort description of a descending sorting key, which is a direct read the second pass accepts. wouldReadInOrderBeUseful already computed the input order and reduced it to a bool, so return it instead (as getInputOrderIfReadInOrderIsUseful) and gate on `direction != 1`, the same value the second pass uses. Because the plan shape of a sound deferral and of one whose second pass rejects the read afterwards is the same, the new test cases also count the reading types of the plan. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ehind a join Covers the interaction of the reverse order FINAL read with ReadFromMerge::requestReadingInOrder and the read-in-order through join pass: with topKThroughJoin disabled, the second pass reads the children of a Merge table in reverse order through the join, and the selected rows must match a read with no optimization at all. topKThroughJoin itself never defers for a Merge table, because it looks for a MergeTree read on the preserved input while a Merge table reads through its own step, so it wins the plan by default. That predates this change and holds for every reading direction, with and without FINAL; the test pins the current behavior rather than changing it. Related to the review comment ClickHouse#111609 (comment). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Measured in CI: with topKThroughJoin enabled, the Merge table plan has one reverse order read, not none. The optimization injects its Sort + Limit because it does not defer for a Merge table, but reading in order then satisfies that injected sort, so the children are read in reverse order anyway and the missing deferral only costs the extra sort step. Reference of https://s3.amazonaws.com/clickhouse-test-reports/PRs/111609/1b48419047f4cb65b18d854abdc2130f04f2e880/fast_test.html Co-Authored-By: Claude Fable 5 <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
1 participant
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.
Closes: #58035
Related: #58361
Second attempt at #58361, this time scoped to
ReplacingMergeTreeonly.When a query with
FINALsorts in reverse order of the sorting key (e.g.ORDER BY key DESC LIMIT n), the read-in-order optimization now applies instead of falling back to a full read.ReplacingSortedAlgorithmlearns aread_in_reversemode: a row with a strictly higher version always replaces the selected one; among rows with equal (or absent) versions, the previously selected row is kept unless the current row comes from a newer data part. This mirrors the "last written row wins" rule of the direct reading order, because in the reverse reading order rows within one part arrive backwards while parts still arrive from the oldest to the newest one. This addresses the correctness concern that stopped the first attempt (#58361 (comment)): duplicate keys inside a single level-0 part now select the same row in both reading directions.The other engines keep the old behavior (no reverse reading with
FINAL), since their merging algorithms rely on the direct order of rows: the sequence of sign rows inCollapsingMergeTree, the order of rows fed to order-dependent aggregate functions inAggregatingMergeTree, etc.On a 110M-row
ReplacingMergeTreetable (two overlapping parts),SELECT x FROM t FINAL ORDER BY x DESC LIMIT 1:Trade-offs: like the already-existing direct-order in-order reads with
FINAL, an in-order plan disables verticalFINALand the splitting of parts ranges into intersecting and non-intersecting ones (the plain parallel read of non-intersecting ranges does not produce key-sorted streams). A full-resultORDER BY key DESCquery without a smallLIMITon a wide or mostly-merged table may therefore regress; the new settingoptimize_read_in_reverse_order_final(default true) allows disabling the optimization, andcompatibilitywith versions before 26.7 restores the previous plans.Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Enable the read-in-order optimization for queries with
FINALthat sort in reverse order of the sorting key onReplacingMergeTreetables. This makes queries such asSELECT ... FROM t FINAL ORDER BY key DESC LIMIT nread only the relevant tail of the data instead of the whole table. Can be disabled with the new settingoptimize_read_in_reverse_order_final.