This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-3463-697abcee75b503fc106f445dae8bcfc401524dcc in repository https://gitbox.apache.org/repos/asf/iceberg-rust.git
commit 1be091a2135a5908537a5b45786ae7207a723aab Author: Marco Kroll <[email protected]> AuthorDate: Sat Oct 17 20:40:52 2026 +0000 fix(transaction): prevent committing to a read-only table (#3463) * Fix committing to a read-only table Prevent committing to a read-only table. This affects for all tables obtained via `StaticTable::into_table`. * Update CHANGELOG.md Co-authored-by: Andrei Tserakhau <[email protected]> * Update `commit` doc string Co-authored-by: Andrei Tserakhau <[email protected]> * Improve readonly test setup --------- Co-authored-by: Marco Kroll <[email protected]> Co-authored-by: Andrei Tserakhau <[email protected]> --- CHANGELOG.md | 1 + crates/iceberg/src/transaction/mod.rs | 63 +++++++++++++++++++++++++++++++++-- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f357a03e5..3076335c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/). * `EncryptedInputFile::metadata()` is now synchronous and derives the plaintext size from key metadata without a storage stat. Remove `.await` from calls to this method. * AGS1 readers now require `StandardKeyMetadata::file_length` and reject missing or invalid lengths without falling back to a storage stat. AGS1-encrypted manifests, manifest lists, and Puffin files written by earlier development builds without this field must be rewritten using a build that can still read them before upgrading. This matches the Java client's read contract. * `iceberg_catalog_loader::BoxedCatalogBuilder` has a new required method, `with_runtime`. Types implementing `CatalogBuilder` get it through the blanket impl; types implementing `BoxedCatalogBuilder` directly (for example, a wrapper around another `Box<dyn BoxedCatalogBuilder>`) must implement it, typically by forwarding the runtime to the builder they wrap. +* `Transaction::commit` now returns `ErrorKind::PreconditionFailed` for read-only tables (`Table::readonly()`, e.g. from `StaticTable::into_table` or `TableBuilder::readonly(true)`), including empty transactions, which previously returned the table unchanged. Java treats an empty transaction on a static table as a no-op. ## [v0.10.1] - 2026-07-28 diff --git a/crates/iceberg/src/transaction/mod.rs b/crates/iceberg/src/transaction/mod.rs index efae9d01b..57520447d 100644 --- a/crates/iceberg/src/transaction/mod.rs +++ b/crates/iceberg/src/transaction/mod.rs @@ -81,7 +81,7 @@ pub use crate::transaction::update_properties::UpdatePropertiesAction; pub use crate::transaction::update_schema::UpdateSchemaAction; pub use crate::transaction::update_statistics::UpdateStatisticsAction; pub use crate::transaction::upgrade_format_version::UpgradeFormatVersionAction; -use crate::{Catalog, TableCommit, TableRequirement, TableUpdate}; +use crate::{Catalog, Error, ErrorKind, TableCommit, TableRequirement, TableUpdate}; /// Table transaction. pub struct Transaction { @@ -171,7 +171,18 @@ impl Transaction { } /// Commit transaction. + /// + /// Returns `ErrorKind::PreconditionFailed` if the table is read-only + /// (`Table::readonly()`), even when the transaction has no actions. pub async fn commit(self, catalog: &dyn Catalog) -> Result<Table> { + if self.table.readonly() { + return Err(Error::new( + ErrorKind::PreconditionFailed, + "Cannot modify a read-only table", + ) + .with_context("table", self.table.identifier().to_string())); + } + if self.actions.is_empty() { // nothing to commit return Ok(self.table); @@ -256,7 +267,7 @@ mod tests { DataContentType, DataFileBuilder, DataFileFormat, Literal, Struct, TableMetadata, TableProperties, }; - use crate::table::Table; + use crate::table::{StaticTable, Table}; use crate::test_utils::{make_encrypted_table, test_runtime}; use crate::transaction::{ApplyTransactionAction, Transaction}; use crate::{Catalog, Error, ErrorKind, TableCreation, TableIdent}; @@ -321,6 +332,18 @@ mod tests { .unwrap() } + async fn make_v2_minimal_readonly_table() -> Table { + let table = make_v2_minimal_table(); + StaticTable::from_metadata( + table.metadata().clone(), + table.identifier().clone(), + FileIO::new_with_memory(), + ) + .await + .unwrap() + .into_table() + } + pub(crate) async fn make_v3_minimal_table_in_catalog(catalog: &impl Catalog) -> Table { let table_ident = TableIdent::from_strs([format!("ns1-{}", uuid::Uuid::new_v4()), "test1".to_string()]) @@ -618,6 +641,42 @@ mod tests { ); assert!(updated_table.encryption_manager().is_some()); } + + #[tokio::test] + async fn test_commit_readonly_table_fails() { + let table = make_v2_minimal_readonly_table().await; + let catalog = MockCatalog::new(); + + // make sure even empty transactions fail + let err = Transaction::new(&table).commit(&catalog).await.unwrap_err(); + + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot modify a read-only table"); + assert!(!err.retryable(), "Error should not be retryable"); + } + + #[tokio::test] + async fn test_commit_readonly_table_with_actions_fails() { + let table = make_v2_minimal_readonly_table().await; + let catalog = MockCatalog::new(); + + let err = create_test_transaction(&table) + .commit(&catalog) + .await + .unwrap_err(); + + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + } + + #[tokio::test] + async fn test_commit_empty_transaction_is_noop() { + let table = make_v2_minimal_table(); + let catalog = MockCatalog::new(); + + let result = Transaction::new(&table).commit(&catalog).await.unwrap(); + + assert_eq!(result.metadata(), table.metadata()); + } } #[cfg(test)]
