shyjsarah commented on code in PR #482:
URL: https://github.com/apache/paimon-rust/pull/482#discussion_r3548520562
##########
crates/paimon/src/table/mod.rs:
##########
@@ -302,6 +342,44 @@ impl Table {
Ok(table)
}
+ pub async fn copy_with_branch(&self, branch_name: &str) -> Result<Self> {
+ let branch = if branch_name.trim().is_empty() {
+ return Err(crate::Error::DataInvalid {
+ message: "Branch name cannot be empty.".to_string(),
+ source: None,
+ });
+ } else {
+ validate_branch_name(branch_name)?;
+ branch_name.to_string()
+ };
+ let schema_manager = if branch == DEFAULT_MAIN_BRANCH {
+ SchemaManager::new(self.file_io.clone(), self.location.clone())
+ } else {
+ SchemaManager::new(self.file_io.clone(),
self.location.clone()).with_branch(&branch)
+ };
+ let schema = schema_manager
+ .latest()
+ .await?
+ .ok_or_else(|| crate::Error::DataInvalid {
+ message: format!("Branch '{branch}' does not exist."),
+ source: None,
+ })?;
+ let mut options = schema.options().clone();
+ options.insert("branch".to_string(), branch.clone());
+ Ok(Self {
+ file_io: self.file_io.clone(),
+ identifier: self.identifier.clone(),
+ location: self.location.clone(),
+ schema: schema.copy_with_replaced_options(options),
+ schema_manager,
+ branch,
+ branch_reference: true,
Review Comment:
Thanks for catching this. Fixed in `847ad3a`.
I centralized the branch-reference write guard in core with
`Table::ensure_not_branch_reference_for_write()` and now enforce it from:
- `WriteBuilder` commit/write/update/delete paths
- `TableCommit` mutating commit paths
- global index build/drop builders that can bypass `WriteBuilder`
I also added coverage for a `$branch_main`-style branch reference to verify
both write-builder commit creation and index build execution fail with the
read-only branch error.
##########
crates/integrations/datafusion/src/sql_context.rs:
##########
@@ -1425,6 +1482,24 @@ impl SQLContext {
}
}
+ fn ensure_main_branch_write_target(name: &ObjectName, operation: &str) ->
DFResult<()> {
+ let object = name
+ .0
+ .last()
+ .and_then(|part| part.as_ident())
+ .map(|ident| ident.value.as_str())
+ .ok_or_else(|| DataFusionError::Plan(format!("Invalid table
reference: {name}")))?;
+ let parsed = parse_object_name(object).map_err(to_datafusion_error)?;
+ if let Some(branch) = parsed.branch() {
+ if branch != DEFAULT_MAIN_BRANCH {
Review Comment:
Fixed in `e49afcb`.
`ensure_main_branch_write_target` now rejects any parsed branch reference,
including `$branch_main`, instead of treating `main` as writable. I also added
`$branch_main` coverage for the intercepted write handlers: `UPDATE`, `DELETE`,
`MERGE INTO`, `TRUNCATE TABLE`, `ALTER TABLE`, and `INSERT OVERWRITE`.
Plain `INSERT INTO ...$branch_main` remains covered through the table
provider write rejection.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
