Repository navigation
revert: compaction row-order suffix relabels (#8400, #9769) - #10440
Merged
Merged
Conversation
wjones127
marked this pull request as ready for review
October 28, 2026 22:00
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
This was referenced Oct 28, 2026
Closed
wkalt
approved these changes
Oct 28, 2026
LuQQiu
added a commit
to LuQQiu/lance
that referenced
this pull request
Oct 29, 2026
Brings in lance-format#10440's revert of the row-order suffix relabel; the output-property changes do not touch the removed code. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
LuQQiu
added a commit
to LuQQiu/lance
that referenced
this pull request
Oct 29, 2026
…paction-grouped Main now carries lance-format#10440, which removed the row-order suffix relabel, so non-adjacent rewrite groups already commit in place and the PreserveRowOrder/ReplaceInPlace switch, the in-place source rebase, the mode carried on RewriteResult and the mode agreement at commit have no purpose and are dropped. What remains is grouped planning: CompactionGroup and DefaultCompactionPlanner::plan_groups, which applies the ordinary candidacy rules through one shared helper, splits at index boundaries and by size, leaves a lone fragment alone unless it must be rewritten by itself, bounds the selected sources by max_source_fragments, and refuses coverage this build cannot remap before anything is written. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
LuQQiu
added a commit
to LuQQiu/lance
that referenced
this pull request
Oct 29, 2026
…dependency Brings the test-fixture initializers from lance-format#10427 and the compaction changes from lance-format#10440; the key-dependency change touches none of those files. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
LuQQiu
added a commit
that referenced
this pull request
Oct 29, 2026
…10445) ## Why An application may know which fragments belong together, while Lance knows which ones actually need compaction. This PR lets the application supply **candidate group boundaries**, including groups whose fragments are not adjacent in the manifest. Lance still selects the useful work. It does not need to understand buckets, partitions, or enterprise clustering specs. ## From candidate groups to final tasks Each disjoint `CompactionGroup` contains: - Candidate input fragments. - The optional clustering ID and opaque metadata its outputs should carry, using #10409's interface. A group is **not an instruction to rewrite every member**. `plan_groups` uses the same candidacy helper as ordinary compaction: - Small fragments can merge within their group. - Healthy full-size fragments stay untouched. - Deletions, overlays or dead columns can justify rewriting a single fragment. - A lone small fragment that would gain nothing is skipped; several small fragments can merge even below the target. Lance splits candidates at index boundaries and by output size, then applies source-fragment/row/byte budgets. No task mixes different groups. Each resulting `TaskData.fragments` is the finalized input list. Unreadable-index safety is checked against the final budget-retained tasks, not healthy members or tasks that will not run. Execution and commit retain their correctness checks. ## Example ```text Table: [A1, B1, A2, B2] Candidates: group A = [A1, A2]; group B = [B1, B2] ``` If A1 and A2 need merging, Lance can put them in one task even though B1 lies between them. If B1 and B2 are already healthy full-size fragments, they produce no task. After executing and committing the A task: ```text [B1, B2, A_new] ``` B1 and B2 retain their IDs, files and metadata. A_new gets a fresh fragment ID and the group's requested output properties. Lance does not interpret what A or B means. ## What stays unchanged Ordinary callers keep their existing entry points and current main's behavior. This PR adds no ordering mode and does not restore the table-wide row-order guarantee removed by #10440. Grouped tasks use the existing execution, Rewrite commit and row-address/index-remapping machinery. Only selected sources are replaced; untouched fragments are not relabelled. Grouped compaction does not promise table-wide scan order. No protobuf or on-disk format change. ## Dependency and reading order Both PRs target main. This branch includes the unmerged #10409 output-property prerequisite; merge #10409 first. The [grouped-only comparison](0c517f5...810cb09) excludes that prerequisite. Start with `CompactionGroup`, shared `candidacy_of`, and `DefaultCompactionPlanner::plan_groups` in `rust/lance/src/dataset/optimize.rs`, then the `grouped_compaction_*` tests. They cover non-adjacent execution/commit, candidate filtering, unreadable coverage and budgets, deferred fragment-reuse recording, and concurrent append/source-change scenarios. --------- Co-authored-by: Claude Fable 5.1 <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
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
Reverts #8400 and #9769.
#8400 makes compaction preserve row order. To do this it relabels every fragment after the first compacted range with fresh IDs, and it makes compaction a retryable conflict with any concurrent
Append(orUpdatethat adds fragments). Under steady ingestion, compaction can then keep retrying and never land. Guaranteeing row order across compaction was previously rejected by the maintainers, so #8400 is rejected as a design.#9769 is built on #8400's metadata-only suffix relabels, so it can't stay without it. It is reverted here and will be redone with its author, without the relabel dependency.
Merge Risk
Door: two-way. Neither PR changed the on-disk format. #8400 shipped only in
v15.0.0-beta.4/beta.5; tables it compacted keep valid, ID-sorted manifests and read normally after the revert.Blast Radius: concurrency (append/compact become compatible again), compaction, fragment reuse history on stable-row-ID tables.
🤖 Generated with Claude Code