mattfaltyn commented on code in PR #3062:
URL: https://github.com/apache/iceberg-rust/pull/3062#discussion_r3860886846
##########
crates/catalog/sql/src/catalog.rs:
##########
@@ -1386,6 +1389,108 @@ mod tests {
new_sql_catalog(warehouse_loc.clone(), Some("iceberg")).await;
}
+ async fn new_commit_error_catalog() -> SqlCatalog {
+ let sql_lite_uri = format!("sqlite:{}", temp_path());
+ sqlx::Sqlite::create_database(&sql_lite_uri).await.unwrap();
+ let catalog = SqlCatalogBuilder::default()
+ .with_storage_factory(Arc::new(LocalFsStorageFactory))
+ .prop("pool.max-connections", "1")
+ .load(
+ "iceberg",
+ HashMap::from_iter([
+ (SQL_CATALOG_PROP_URI.to_string(), sql_lite_uri),
+ (SQL_CATALOG_PROP_WAREHOUSE.to_string(), temp_path()),
+ ]),
+ )
+ .await
+ .unwrap();
+
+ catalog
+ .connection
+ .execute("PRAGMA foreign_keys = ON")
+ .await
+ .unwrap();
+ // This deferred constraint lets an INSERT succeed while COMMIT fails.
+ catalog
+ .connection
+ .execute("CREATE TABLE parent(id INTEGER PRIMARY KEY)")
+ .await
+ .unwrap();
+ catalog
+ .connection
+ .execute(
+ "CREATE TABLE child(parent_id INTEGER REFERENCES parent(id) \
+ DEFERRABLE INITIALLY DEFERRED)",
+ )
+ .await
+ .unwrap();
+
+ catalog
+ }
+
+ #[tokio::test]
+ async fn test_execute_returns_commit_error() {
+ let catalog = new_commit_error_catalog().await;
+
+ // Make the public namespace operation insert a child row whose
deferred
+ // foreign-key constraint succeeds during execution but fails at
commit.
+ let trigger = format!(
+ "CREATE TRIGGER fail_namespace_commit
+ AFTER INSERT ON {NAMESPACE_TABLE_NAME}
+ BEGIN INSERT INTO child VALUES (1); END"
+ );
+ catalog.connection.execute(trigger.as_str()).await.unwrap();
+
+ let failed_namespace = NamespaceIdent::new("failed".into());
+ let error = catalog
+ .create_namespace(&failed_namespace, HashMap::new())
+ .await
+ .unwrap_err();
+ assert_eq!(error.kind(), ErrorKind::Unexpected);
+ assert!(!catalog.namespace_exists(&failed_namespace).await.unwrap());
+
+ // A valid relationship confirms that successful transactions still
commit.
+ catalog
+ .connection
+ .execute("INSERT INTO parent VALUES (1)")
+ .await
+ .unwrap();
+ let committed_namespace = NamespaceIdent::new("committed".into());
+ catalog
+ .create_namespace(&committed_namespace, HashMap::new())
+ .await
+ .unwrap();
+ assert!(
+ catalog
+ .namespace_exists(&committed_namespace)
+ .await
+ .unwrap()
+ );
+ }
+
+ #[tokio::test]
+ async fn test_execute_with_external_transaction_returns_commit_error() {
Review Comment:
Agreed—removed the SQLite-only external-transaction test. The remaining
regression exercises commit-error propagation through public `create_namespace`
and verifies rollback via `namespace_exists`.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
