Skip to content

feat: expose staged index segment transactions#6806

Open
ragnorc wants to merge 2 commits into
lance-format:mainfrom
ModernRelay:ragnorc/two-phase-index-segments
Open

feat: expose staged index segment transactions#6806
ragnorc wants to merge 2 commits into
lance-format:mainfrom
ModernRelay:ragnorc/two-phase-index-segments

Conversation

@ragnorc
Copy link
Copy Markdown
Contributor

@ragnorc ragnorc commented May 16, 2026

Summary

  • Add DatasetIndexExt::build_existing_index_segments_transaction to prepare an Operation::CreateIndex transaction for existing index segments without committing.
  • Refactor commit_existing_index_segments to reuse the staged transaction path.
  • Add coverage that verifies staging does not publish the index and the returned transaction can be committed via CommitBuilder.

Tests

  • cargo fmt --all
  • cargo test -p lance existing_index_segments -- --nocapture
  • cargo test -p lance test_vector_execute_uncommitted_segments_commit_without_staging -- --nocapture
  • External crate cargo check against the new public API

Fixes #6666

Co-authored-by: Ragnor Comerford <ragnorc@users.noreply.github.com>
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@github-actions github-actions Bot added the enhancement New feature or request label May 16, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b0d5f35c19

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +315 to +320
async fn build_existing_index_segments_transaction(
&self,
index_name: &str,
column: &str,
segments: Vec<impl IntoIndexSegment + Send>,
) -> Result<Transaction>;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add a default body for new DatasetIndexExt method

Adding build_existing_index_segments_transaction as a required method on the public DatasetIndexExt trait is a breaking API change for downstream crates that implement this trait: they will fail to compile immediately after upgrading, even if they never use this new functionality. Because DatasetIndexExt is publicly exported, this should be introduced with a default implementation (or via a new extension trait) to preserve semver compatibility.

Useful? React with 👍 / 👎.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 16, 2026

Codecov Report

❌ Patch coverage is 98.55072% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance/src/index.rs 98.55% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose build_index_metadata_from_segments (or commit_existing_index_segments) for two-phase vector-index commits outside the lance crate

2 participants