Skip to content

refactor: Deduplicate apply_deletions and promote KeyExistenceFilter to write module#6384

Open
ztorchan wants to merge 1 commit intolance-format:mainfrom
ztorchan:cleanup-redundant-code
Open

refactor: Deduplicate apply_deletions and promote KeyExistenceFilter to write module#6384
ztorchan wants to merge 1 commit intolance-format:mainfrom
ztorchan:cleanup-redundant-code

Conversation

@ztorchan
Copy link
Copy Markdown
Contributor

@ztorchan ztorchan commented Apr 2, 2026

Motivation

The apply_deletions function was duplicated across 4 files (delete.rs, update.rs, merge_insert.rs, and merge_insert/exec.rs) with nearly identical logic. Additionally, KeyExistenceFilter and related types were nested deep inside merge_insert/inserted_rows.rs, despite being used by transaction.rs at a higher abstraction level.

Changes

  1. Extract apply_deletions into a shared public function in write.rs

    • Added pub async fn apply_deletions(dataset, removed_row_addrs) to write.rs as the single canonical implementation.
    • Removed the 4 duplicate implementations from delete.rs, update.rs, merge_insert.rs, and merge_insert/exec.rs.
    • Updated all call sites to use the shared function.
  2. Promote KeyExistenceFilter to write/key_existence_filter.rs

    • Moved inserted_rows.rs from merge_insert/ to write/ as key_existence_filter.rs, making it a direct submodule of write.
    • Deleted the original merge_insert/inserted_rows.rs file.
    • Updated all import paths in merge_insert.rs, merge_insert/exec/write.rs, and transaction.rs to reference the new location.
  3. Cleanup

    • Removed unused imports (BTreeMap, StreamExt, RoaringTreemap, Fragment, etc.) from files that no longer need them after the refactor.

Impact

  • No functional changes. All existing tests pass (delete, update, merge_insert, key_existence_filter, transaction).
  • Reduces code duplication (~50 lines × 4 copies → 1 shared function).
  • Simplifies the import path for KeyExistenceFilter (e.g., write::key_existence_filter::KeyExistenceFilter instead of write::merge_insert::inserted_rows::KeyExistenceFilter).

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

ACTION NEEDED
Lance follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification.

For details on the error please inspect the "PR Title Check" action.

@ztorchan ztorchan changed the title Refactor: Deduplicate apply_deletions and promote KeyExistenceFilter to write module 人、efactor: Deduplicate apply_deletions and promote KeyExistenceFilter to write module Apr 2, 2026
@ztorchan ztorchan changed the title 人、efactor: Deduplicate apply_deletions and promote KeyExistenceFilter to write module refactor: Deduplicate apply_deletions and promote KeyExistenceFilter to write module Apr 2, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 2, 2026

Codecov Report

❌ Patch coverage is 93.75000% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance/src/dataset/write/merge_insert.rs 0.00% 0 Missing and 1 partial ⚠️
rust/lance/src/dataset/write/update.rs 50.00% 0 Missing and 1 partial ⚠️

📢 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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant