Skip to content

fix(catalog-sql): forward catalog props to FileIO#2528

Open
kszucs wants to merge 1 commit into
apache:mainfrom
kszucs:fix-sql-catalog-fileio-props
Open

fix(catalog-sql): forward catalog props to FileIO#2528
kszucs wants to merge 1 commit into
apache:mainfrom
kszucs:fix-sql-catalog-fileio-props

Conversation

@kszucs
Copy link
Copy Markdown
Member

@kszucs kszucs commented May 29, 2026

Which issue does this PR close?

  • 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:

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.

Storage-backend keys set on SqlCatalog were not reaching the FileIO
because SqlCatalog::new built it without forwarding props, causing
401s on authenticated writes. Matches the RestCatalog wiring.
@kszucs kszucs force-pushed the fix-sql-catalog-fileio-props branch from 4941490 to 65eb9b9 Compare May 29, 2026 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant