Skip to content

refactor(sql): derive catalog configuration from properties - #3105

Open
blackmwk wants to merge 2 commits into
apache:mainfrom
blackmwk:ir-3099-sql-properties
Open

refactor(sql): derive catalog configuration from properties#3105
blackmwk wants to merge 2 commits into
apache:mainfrom
blackmwk:ir-3099-sql-properties

Conversation

@blackmwk

@blackmwk blackmwk commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

What changes are included in this PR?

  • Replace the intermediate SQL catalog config with crate-private, typed SqlCatalogProperties derived from the merged property map.
  • Keep URI, warehouse, and bind-style builder values as builder-only fallbacks while preserving load-property precedence and the legacy bind-style alias.
  • Parse bind style and schema version into their domain types during property derivation with compatible validation errors.
  • Forward the complete merged property map unchanged to pool configuration, FileIO, and KMS.
  • Add focused coverage for typed parsing, defaults, precedence, aliases, invalid values, and raw property forwarding.

Are these changes tested?

  • cargo test -p iceberg-catalog-sql --lib (51 passed)
  • cargo clippy -p iceberg-catalog-sql --all-targets -- -D warnings
  • cargo public-api -p iceberg-catalog-sql --all-features -ss | diff - crates/catalog/sql/public-api.txt
  • cargo fmt --all -- --check
  • git diff --check

AI Disclosure

This change was developed with assistance from OpenAI Codex. The contributor reviewed the resulting diff and test output.

@blackmwk
blackmwk force-pushed the ir-3099-sql-properties branch from 2bda348 to 8b2ba53 Compare September 4, 2026 03:16
Use the shared property derive for SQL catalog settings while preserving builder precedence, bind-style aliases, schema validation, and forwarded pool properties.

Tracks [apache#3099](apache#3099).

Generated-by: Codex
@blackmwk
blackmwk force-pushed the ir-3099-sql-properties branch from 8b2ba53 to 4718d5b Compare September 4, 2026 03:27
Comment thread crates/catalog/sql/src/catalog.rs Outdated
Comment thread crates/catalog/sql/src/catalog.rs Outdated
Comment thread crates/catalog/sql/src/catalog.rs Outdated
@blackmwk
blackmwk marked this pull request as ready for review September 4, 2026 08:06
@blackmwk

blackmwk commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

I think this pr is ready for review, cc @CTTY @laskoviymishka

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.

refactor(sql): derive catalog configuration from properties

1 participant