Skip to content

refactor: unify segment build flow for index creation#6279

Open
Xuanwo wants to merge 4 commits intomainfrom
xuanwo/segment-unified-pr1
Open

refactor: unify segment build flow for index creation#6279
Xuanwo wants to merge 4 commits intomainfrom
xuanwo/segment-unified-pr1

Conversation

@Xuanwo
Copy link
Copy Markdown
Collaborator

@Xuanwo Xuanwo commented Mar 24, 2026

This PR routes all index creation through the same execute_uncommitted() -> segment builder -> commit pipeline, keeps vector multi-input finalize behavior intact, and limits non-vector indices to single-input identity segment build for now.

It also updates segment-related docs and comments to use the new segment terminology consistently, instead of reinforcing the old delta-index model.

@github-actions
Copy link
Copy Markdown
Contributor

PR Review

Clean refactoring that eliminates the uses_segment_commit_path branch and routes all index types through the same execute_uncommitted → segment builder → commit pipeline. The identity plan shortcut for single-segment inputs is a good way to generalize without adding overhead for non-vector indices.

One thing to consider

is_vector_segment relies on type URL suffix matching (create.rs new helper):

fn is_vector_segment(segment: &IndexMetadata) -> bool {
    segment.index_details.as_ref()
        .is_some_and(|details| details.type_url.ends_with("VectorIndexDetails"))
}

This works today but is fragile — if the protobuf type URL format changes or a new vector index type is added with a different message name, this will silently misclassify segments and hit the multi-input rejection error. Consider matching on an enum/variant from IndexType or the protobuf full name constant instead, if one is available. If not, a comment noting the coupling would help future readers.

Looks good otherwise

  • Identity plan logic in plan() and build() is consistent and correct.
  • target_segment_bytes == 0 validation is a nice addition.
  • Test coverage for scalar identity build, multi-input scalar rejection, and scalar commit metadata is thorough.
  • Terminology updates (delta → segment) are consistent throughout.

🤖 Generated with Claude Code

@Xuanwo Xuanwo force-pushed the xuanwo/segment-unified-pr1 branch from 9153063 to 27c06ff Compare March 24, 2026 17:04
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 24, 2026

Codecov Report

❌ Patch coverage is 91.19497% with 14 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance/src/index/create.rs 91.19% 11 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

Xuanwo added 2 commits March 26, 2026 03:20
…d-pr1

# Conflicts:
#	rust/lance-index/src/traits.rs
#	rust/lance-index/src/types.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants