Skip to content

fix: support REST branches with distinct catalog identities #1109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JingsongLi merged 1 commit into from
Oct 12, 2026
Merged

fix: support REST branches with distinct catalog identities #1109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions crates/paimon/src/table/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -903,12 +903,12 @@ impl Table {
// Catalog branches may have no schema directory in the data store.
// Load their schema, identity and FileIO through the same catalog.
let identifier = env.identifier().with_branch(&branch)?;
let mut table = env.get_table(&identifier).await?;
if table.location() != self.location()
|| table
.rest_env
.as_ref()
.is_none_or(|branch_env| branch_env.uuid() != env.uuid())
let (current, mut table) = tokio::try_join!(
env.current_table_identity_checked(),
env.get_table(&identifier)
)?;
Comment on lines +906 to +909

@JingsongLi JingsongLi Oct 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Recheck the source identity after loading the target

These concurrent GETs can observe different catalog generations. For an external table re-registered at the same path, the source request can return the loaded v1 UUID, then the target request return v2 after re-registration. Both location checks pass, and the returned Table/RESTEnv silently adopt the replacement UUID/schema. This even reproduces with a loaded main handle calling copy_with_branch("main"): using the real HTTP mock's set_table_uuid_after_calls(..., "replacement-v2", 1), all 16 attempts return v2 from a handle loaded as v1. The baseline switch never adopts v2 in the same probe. Stable external paths are a supported catalog registration model, so matching paths do not establish a shared generation. Please bracket the target resolution with another source UUID/path check, or provide an equivalent identity association; a temporary post-target check fixes the probe and preserves the distinct-branch behavior. Add a regression for re-registration during the switch, in addition to the already-stale-source test.

if current.path.as_deref() != Some(self.location())
|| table.location() != self.location()
{
return Err(crate::Error::DataInvalid {
message: format!(
Expand Down
24 changes: 22 additions & 2 deletions crates/paimon/src/table/rest_env.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ impl RESTEnv {
schema_id: i64,
fields: &[crate::spec::DataField],
) -> Result<GetTableResponse> {
let response = self.api.get_table(&self.identifier).await?;
let response = self.current_table_identity_checked().await?;
let name = self.identifier.full_name();
let same = |what: &str, loaded: String, now: Option<String>| match now {
Some(now) if now == loaded => Ok(()),
Expand All @@ -168,7 +168,6 @@ impl RESTEnv {
source: None,
}),
};
same("uuid", self.uuid.clone(), response.id.clone())?;
same(
"schema",
schema_id.to_string(),
Expand All @@ -194,6 +193,27 @@ impl RESTEnv {
Ok(response)
}

/// Refused unless the identifier still resolves to this catalog entity.
pub(crate) async fn current_table_identity_checked(&self) -> Result<GetTableResponse> {
let response = self.api.get_table(&self.identifier).await?;
let name = self.identifier.full_name();
match response.id.as_deref() {
Some(uuid) if uuid == self.uuid => {}
uuid => {
return Err(crate::Error::DataInvalid {
message: format!(
"table '{name}' now resolves to uuid {}, not the {} this handle was loaded \
with; re-load the table before reading it",
uuid.unwrap_or("nothing the server reports"),
self.uuid
),
source: None,
});
}
}
Ok(response)
}

/// Get the table identifier.
pub fn identifier(&self) -> &Identifier {
&self.identifier
Expand Down
25 changes: 13 additions & 12 deletions crates/paimon/tests/rest_catalog_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5211,7 +5211,7 @@ async fn test_rest_catalog_manages_policies_end_to_end() {
}

#[tokio::test]
async fn test_branch_write_copy_loads_catalog_schema_without_schema_files() {
async fn test_branch_write_copy_loads_catalog_schema_with_distinct_identity() {
let ctx = setup_catalog(vec!["default"]).await;
let path = "memory:/branch-write";
ctx.server
Expand All @@ -5226,7 +5226,8 @@ async fn test_branch_write_copy_loads_catalog_schema_without_schema_files() {
.add_table_with_schema("default", "t$branch_dev", branch_schema.clone(), path);
ctx.server
.set_table_schema_id("default", "t$branch_dev", branch_schema, 7);
ctx.server.set_table_uuid("default", "t$branch_dev", "t");
ctx.server
.set_table_uuid("default", "t$branch_dev", "branch-t");
let main = ctx
.catalog
.get_table(&Identifier::new("default", "t"))
Expand Down Expand Up @@ -5264,27 +5265,26 @@ async fn test_branch_write_copy_loads_catalog_schema_without_schema_files() {
}

#[tokio::test]
async fn test_branch_write_copy_rejects_replaced_identity_or_location() {
async fn test_branch_write_copy_rejects_replaced_source_identity_or_location() {
let ctx = setup_catalog(vec!["default"]).await;
let path = "memory:/branch-identity";
ctx.server
.add_table_with_schema("default", "t", test_schema(), path);
ctx.server
.add_table_with_schema("default", "t$branch_dev", test_schema(), path);
ctx.server.set_table_uuid("default", "t$branch_dev", "t");
ctx.server
.set_table_uuid("default", "t$branch_dev", "branch-t");
let main = ctx
.catalog
.get_table(&Identifier::new("default", "t"))
.await
.unwrap();
main.copy_with_branch("dev").await.unwrap();
let branch = main.copy_with_branch("dev").await.unwrap();
ctx.server
.set_table_uuid("default", "t$branch_dev", "recreated-table");
let error = main.copy_with_branch("dev").await.unwrap_err();
.set_table_uuid("default", "t$branch_dev", "recreated-branch");
let error = branch.copy_with_branch("main").await.unwrap_err();
assert!(
error
.to_string()
.contains("no longer belongs to the loaded table"),
error.to_string().contains("now resolves to uuid"),
"{error}"
);
ctx.server.add_table_with_schema(
Expand All @@ -5293,8 +5293,9 @@ async fn test_branch_write_copy_rejects_replaced_identity_or_location() {
test_schema(),
"memory:/replacement",
);
ctx.server.set_table_uuid("default", "t$branch_dev", "t");
let error = main.copy_with_branch("dev").await.unwrap_err();
ctx.server
.set_table_uuid("default", "t$branch_dev", "branch-t");
let error = branch.copy_with_branch("main").await.unwrap_err();
assert!(
error
.to_string()
Expand Down
Loading