mattfaltyn opened a new issue, #2846:
URL: https://github.com/apache/iceberg-rust/issues/2846
### 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:
```text
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
1. Check out revision `db4f6091850814b83989721afe12aa9e4406d6b3`.
2. Initialize an otherwise valid in-memory SQL catalog with a non-numeric
pool property:
```rust
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(())
}
```
3. 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
--
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]
