Skip to content

SQL catalog panics on invalid pool property values #2846

Description

@mattfaltyn

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

  1. Check out revision db4f6091850814b83989721afe12aa9e4406d6b3.

  2. 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(())
}
  1. 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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions