Skip to content

feat(namespace-dir): implement rename_table for DirectoryNamespace#6975

Open
Ar-maan05 wants to merge 5 commits into
lance-format:mainfrom
Ar-maan05:feat/dir-namespace-rename-table
Open

feat(namespace-dir): implement rename_table for DirectoryNamespace#6975
Ar-maan05 wants to merge 5 commits into
lance-format:mainfrom
Ar-maan05:feat/dir-namespace-rename-table

Conversation

@Ar-maan05

Copy link
Copy Markdown
Contributor

Summary

Implements rename_table for DirectoryNamespace, which previously fell back to the trait's "not implemented" default. Closes #6960

Behavior by mode:

  • Dir-only mode (manifest disabled): Only the root namespace exists, so renames are within it. Validates the new name, verifies the source table exists, rejects an existing destination, then copies the table directory to the new name and removes the source. Cross-namespace renames return Unsupported.
  • Manifest mode: Delegates to ManifestNamespace::rename_table, which resolves the source entry from the manifest, validates the destination namespace exists, picks a destination directory consistent with create_table, copies the files, registers the new manifest entry before dropping the old one (so the table stays reachable under at least one name if a later step fails), and removes the source directory. Supports cross-namespace moves via new_namespace_id.

Also adds a shared copy_dir_all helper for recursive directory copies (works on both local FS and object stores).

Error handling

  • InvalidInput : empty new_table_name
  • TableNotFound : source table does not exist
  • TableAlreadyExists : destination already exists
  • Unsupported : cross-namespace rename without manifest

Test plan

  • cargo fmt --all
  • cargo clippy -p lance-namespace-impls --tests -- -D warnings (clean)
  • cargo test -p lance-namespace-impls : 190 unit tests pass, including 8 new rename_table tests covering both modes:
    • basic rename moves data and the new name is readable (correct row count)
    • destination-conflict rejection
    • source-not-found
    • empty-name rejection
    • cross-namespace rejection without manifest
    • cross-namespace move with manifest

@claude claude Bot left a comment

Copy link
Copy Markdown

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 28, 2026
@codecov

codecov Bot commented May 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.61710% with 36 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance-namespace-impls/src/dir/manifest.rs 40.42% 26 Missing and 2 partials ⚠️
rust/lance-namespace-impls/src/dir.rs 96.39% 8 Missing ⚠️

📢 Thoughts on this report? Let us know!

@Ar-maan05

Copy link
Copy Markdown
Contributor Author

Merged the latest main to resolve the conflict.

The only collision was the lance_namespace::models import list in lance-namespace-impls/src/dir.rs : #6886 (table tag ops) added imports right next to this PR's RenameTableRequest/RenameTableResponse. Resolved as the union of both sides, so nothing was dropped (RenameTable* plus TagContents/ UpdateTableTag* are all retained). The rename_table implementation itself merged cleanly with no changes.

Verified locally:
cargo clippy -p lance-namespace-impls --features lance/protoc --tests -- -D warnings -> clean.

@Ar-maan05

Copy link
Copy Markdown
Contributor Author

Friendly ping! All checks passing, happy to make any changes if needed.

@github-actions github-actions Bot added the A-namespace Namespace impls label Jun 11, 2026
@Ar-maan05

Copy link
Copy Markdown
Contributor Author

I have rebased the branch with the latest main and resolved all merge conflicts. let me know if any changes are needed.

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

Labels

A-namespace Namespace impls enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(namespace-dir): implement rename_table for DirectoryNamespace

1 participant