Apache Iceberg Rust version
None
Describe the bug
SqlCatalogBuilder::load panics when one of the following SQL connection-pool properties cannot be parsed:
pool.max-connections
pool.idle-timeout
pool.test-before-acquire
On current main (db4f6091850814b83989721afe12aa9e4406d6b3, workspace version 0.10.0), these properties are parsed using parse().unwrap() in crates/catalog/sql/src/catalog.rs. A malformed configuration value therefore causes a panic instead of returning an error through the builder's Result.
For example, setting pool.max-connections=not-a-number panics with:
called `Result::unwrap()` on an `Err` value: ParseIntError { kind: InvalidDigit }
The builder already reports other invalid configuration, such as an invalid sql_bind_style, using ErrorKind::DataInvalid. The pool properties should behave consistently.
To Reproduce
-
Check out revision db4f6091850814b83989721afe12aa9e4406d6b3.
-
Initialize an otherwise valid in-memory SQL catalog with a non-numeric pool property:
use std::{collections::HashMap, sync::Arc};
use iceberg::{CatalogBuilder, Result};
use iceberg_catalog_sql::{
SQL_CATALOG_PROP_BIND_STYLE, SQL_CATALOG_PROP_URI,
SQL_CATALOG_PROP_WAREHOUSE, SqlBindStyle, SqlCatalogBuilder,
};
use iceberg_storage_opendal::OpenDalStorageFactory;
#[tokio::main]
async fn main() -> Result<()> {
let props = HashMap::from([
(
SQL_CATALOG_PROP_URI.to_string(),
"sqlite::memory:".to_string(),
),
(
SQL_CATALOG_PROP_WAREHOUSE.to_string(),
"memory://warehouse".to_string(),
),
(
SQL_CATALOG_PROP_BIND_STYLE.to_string(),
SqlBindStyle::QMark.to_string(),
),
(
"pool.max-connections".to_string(),
"not-a-number".to_string(),
),
]);
let _catalog = SqlCatalogBuilder::default()
.with_storage_factory(Arc::new(OpenDalStorageFactory::Memory))
.load("repro", props)
.await?;
Ok(())
}
- Run the program.
The process exits with status 101 and panics at crates/catalog/sql/src/catalog.rs:284 with ParseIntError { kind: InvalidDigit }.
Changing the property value to "1" initializes the catalog successfully.
Expected behavior
SqlCatalogBuilder::load should return an Err with ErrorKind::DataInvalid that identifies the invalid property and value. Invalid user-provided configuration should not panic or unwind the embedding process.
The same behavior should apply to all three pool properties.
Willingness to contribute
I can contribute a fix for this bug independently
Apache Iceberg Rust version
None
Describe the bug
SqlCatalogBuilder::loadpanics when one of the following SQL connection-pool properties cannot be parsed:pool.max-connectionspool.idle-timeoutpool.test-before-acquireOn current
main(db4f6091850814b83989721afe12aa9e4406d6b3, workspace version0.10.0), these properties are parsed usingparse().unwrap()incrates/catalog/sql/src/catalog.rs. A malformed configuration value therefore causes a panic instead of returning an error through the builder'sResult.For example, setting
pool.max-connections=not-a-numberpanics with:The builder already reports other invalid configuration, such as an invalid
sql_bind_style, usingErrorKind::DataInvalid. The pool properties should behave consistently.To Reproduce
Check out revision
db4f6091850814b83989721afe12aa9e4406d6b3.Initialize an otherwise valid in-memory SQL catalog with a non-numeric pool property:
The process exits with status 101 and panics at
crates/catalog/sql/src/catalog.rs:284withParseIntError { kind: InvalidDigit }.Changing the property value to
"1"initializes the catalog successfully.Expected behavior
SqlCatalogBuilder::loadshould return anErrwithErrorKind::DataInvalidthat identifies the invalid property and value. Invalid user-provided configuration should not panic or unwind the embedding process.The same behavior should apply to all three pool properties.
Willingness to contribute
I can contribute a fix for this bug independently