fix(persistence): reject a rename that would move an entity across catalogs - #5305
Open
flyingImer wants to merge 2 commits into
Open
fix(persistence): reject a rename that would move an entity across catalogs#5305flyingImer wants to merge 2 commits into
flyingImer wants to merge 2 commits into
Conversation
dimas-b
reviewed
Aug 17, 2026
dimas-b
left a comment
dimas-b
left a comment
Contributor
There was a problem hiding this comment.
Good catch! Thanks, @flyingImer !
…talogs renameEntity derives the target catalog id from the head of newCatalogPath for its name-collision pre-check, then persists the entity with only parentId re-pointed. A cross-catalog rename would store a row whose parent_id lives in the new catalog while its catalog_id still names the old one, and by-name lookups match that row from neither side. Moving an entity between catalogs is not a one-field change: its children, its grant records and its policy mappings all record the catalog it started in, and renameEntity writes none of them. Reject the call rather than half-perform it. No API can express a cross-catalog rename today, so the new test drives the manager directly, from the shared metastore-manager fixture.
An empty catalogPath is the documented shape for a top-level entity, so the previous check let any non-empty newCatalogPath through whenever the source path was null or empty. That is the same inconsistency this change is about: the entity keeps catalog id 0 while its parent_id points inside a catalog. Require a non-empty catalogPath whose head matches the destination, and cover it with a principal-role case in the shared fixture. Reverting only the predicate makes that assertion fail.
flyingImer
force-pushed
the
fix/rename-entity-cross-catalog-guard
branch
from
August 18, 2026 04:15
666b858 to
1835475
Compare
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
2 participants
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.
Problem
renameEntity accepts a destination path, but it does not support moving an entity across catalogs. Before this change, nothing enforced that restriction.
If newCatalogPath is rooted in a different catalog, the rename can persist an inconsistent entity: its parentId is changed to a parent in the destination catalog, while its catalogId still points to the source catalog.
That entity is then no longer addressable by its expected path. A lookup under the old path uses the old parent and misses it, while a lookup under the new path uses the destination catalog id and misses it there as well.
The current REST table-rename surface cannot express this case because its source and destination are Iceberg TableIdentifiers, which contain a namespace and name but no catalog. However, renameEntity itself does not state or enforce that restriction, so a direct or future caller can create invalid persisted state.
Root cause
The rename implementations treat newCatalogPath as a re-parenting operation within the existing catalog.
When checking for a name collision at the destination, they derive the target catalog id from the first element of newCatalogPath. But when the rename is persisted, only the entity's parent is re-pointed; its existing catalogId is retained.
That is valid for moving an entity between namespaces in the same catalog, but not across catalogs.
A cross-catalog move would require more than changing the entity's parent. Descendants, grant records, and policy mappings can also retain references to the catalog the entity belongs to. renameEntity does not migrate that state, so accepting a cross-catalog destination would only partially perform the move.
The fix
Reject a rename when both the current and destination paths are present and their root catalog ids differ.
The validation is added consistently to: AtomicOperationMetaStoreManager, TransactionalMetaStoreManagerImpl, NoSqlMetaStoreManager
Same-catalog renames and re-parenting continue to use the existing behavior.
Checklist
CHANGELOG.md(if needed)site/content/in-dev/unreleased(if needed)