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

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions