You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "kszucs (via GitHub)" <gi...@apache.org> on 2026/05/29 12:20:08 UTC
[PR] fix(catalog-sql): forward catalog props to FileIO [iceberg-rust]
kszucs opened a new pull request, #2528:
URL: https://github.com/apache/iceberg-rust/pull/2528
## Which issue does this PR close?
<!-- No tracking issue; surfaced while wiring an `iceberg-storage-opendal::OpenDalStorageFactory::Hf` storage backend behind `SqlCatalog`. Happy to file a follow-up issue if preferred. -->
- Closes #.
## What changes are included in this PR?
Storage-backend keys set on `SqlCatalog` (e.g. authentication tokens, region overrides) were not reaching the `FileIO`, because `SqlCatalog::new` built the `FileIO` from the `StorageFactory` alone, dropping the props on the floor. Authenticated storage backends then saw empty config and rejected writes with 401s.
The fix forwards the catalog props into the `FileIO`:
```rust
let fileio = FileIOBuilder::new(factory)
.with_props(config.props.clone())
.build();
```
This brings `SqlCatalog` into parity with the `RestCatalog`, which already forwards props through `FileIOBuilder::with_props`. Storage backends ignore keys they don't recognize, so catalog-only props (`pool.*` etc.) remain harmless.
## Are these changes tested?
Yes — added unit test `test_storage_props_propagate_to_file_io` in `crates/catalog/sql/src/catalog.rs` which:
- Builds a `SqlCatalog` with two storage-backend props (one S3-shaped, one HF-shaped) in the load map.
- Asserts both keys are present in `catalog.fileio.config().props()`.
The test fails on `main` (the props lookup returns `None`) and passes with this change. Verified locally by reverting the one-line change in the source, observing the assertion failure, then restoring.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix(catalog-sql): forward catalog props to FileIO [iceberg-rust]
Posted by "CTTY (via GitHub)" <gi...@apache.org>.
CTTY merged PR #2528:
URL: https://github.com/apache/iceberg-rust/pull/2528
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix(catalog-sql): forward catalog props to FileIO [iceberg-rust]
Posted by "CTTY (via GitHub)" <gi...@apache.org>.
CTTY commented on code in PR #2528:
URL: https://github.com/apache/iceberg-rust/pull/2528#discussion_r3357763697
##########
crates/catalog/sql/src/catalog.rs:
##########
@@ -1185,6 +1189,33 @@ mod tests {
new_sql_catalog(warehouse_loc.clone(), Some("iceberg")).await;
}
+ /// Regression test: storage-backend props set on the catalog must reach
+ /// the FileIO; otherwise authenticated backends fail with 401s on writes.
Review Comment:
nit: this should not be rustdoc? I think a normal `//` would be better
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix(catalog-sql): forward catalog props to FileIO [iceberg-rust]
Posted by "kszucs (via GitHub)" <gi...@apache.org>.
kszucs commented on code in PR #2528:
URL: https://github.com/apache/iceberg-rust/pull/2528#discussion_r3372915215
##########
crates/catalog/sql/src/catalog.rs:
##########
@@ -1185,6 +1189,33 @@ mod tests {
new_sql_catalog(warehouse_loc.clone(), Some("iceberg")).await;
}
+ /// Regression test: storage-backend props set on the catalog must reach
+ /// the FileIO; otherwise authenticated backends fail with 401s on writes.
Review Comment:
Updated.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org