refactor(sqllogictest): use tagged enum for EngineConfig with CatalogConfig

- Replace EngineConfig struct with tagged enum for better type safety
- Add CatalogConfig struct with catalog_type and props fields
- Remove unused _name parameter from DataFusionEngine::new()
- Prepare structure for future catalog loader integration

Author: AndreaBozzo

crates/sqllogictest/src/engine/datafusion.rs

@@ -28,7 +28,7 @@ use iceberg::{Catalog, CatalogBuilder, NamespaceIdent, TableCreation};
 use iceberg_datafusion::IcebergCatalogProvider;
 use indicatif::ProgressBar;
 
-use crate::engine::{EngineConfig, EngineRunner, run_slt_with_runner};
+use crate::engine::{CatalogConfig, EngineRunner, run_slt_with_runner};
 use crate::error::Result;
 
 pub struct DataFusionEngine {
@@ -58,12 +58,12 @@ impl EngineRunner for DataFusionEngine {
 }
 
 impl DataFusionEngine {
-    pub async fn new(_name: &str, config: &EngineConfig) -> Result<Self> {
+    pub async fn new(catalog_config: Option<CatalogConfig>) -> Result<Self> {
         let session_config = SessionConfig::new()
             .with_target_partitions(4)
             .with_information_schema(true);
         let ctx = SessionContext::new_with_config(session_config);
-        ctx.register_catalog("default", Self::create_catalog(&config.extra).await?);
+        ctx.register_catalog("default", Self::create_catalog(catalog_config.as_ref()).await?);
 
         Ok(Self {
             test_data_path: PathBuf::from("testdata"),
@@ -72,10 +72,10 @@ impl DataFusionEngine {
     }
 
     async fn create_catalog(
-        _extra: &HashMap<String, toml::Value>,
+        _catalog_config: Option<&CatalogConfig>,
     ) -> anyhow::Result<Arc<dyn CatalogProvider>> {
-        // TODO: support dynamic catalog configuration
-        //  See: https://github.com/apache/iceberg-rust/issues/1780
+        // TODO: Use catalog_config to load different catalog types via iceberg-catalog-loader
+        // See: https://github.com/apache/iceberg-rust/issues/1780
         let catalog = MemoryCatalogBuilder::default()
             .load(
                 "memory",

crates/sqllogictest/src/engine/mod.rs

@@ -27,34 +27,37 @@ use sqllogictest::{AsyncDB, MakeConnection, Runner, parse_file};
 use crate::engine::datafusion::DataFusionEngine;
 use crate::error::{Error, Result};
 
-/// Supported engine types for sqllogictest
-#[derive(Debug, Clone, Deserialize, PartialEq, Eq)]
-#[serde(rename_all = "lowercase")]
-pub enum EngineType {
-    Datafusion,
-}
-
-/// Configuration for a single engine instance
+/// Configuration for the catalog used by an engine
 #[derive(Debug, Clone, Deserialize)]
-pub struct EngineConfig {
-    /// The type of engine
+pub struct CatalogConfig {
+    /// Catalog type: "memory", "rest", "glue", "hms", "s3tables", "sql"
     #[serde(rename = "type")]
-    pub engine_type: EngineType,
+    pub catalog_type: String,
+    /// Catalog properties passed to the catalog loader
+    #[serde(default)]
+    pub props: HashMap<String, String>,
+}
 
-    /// Additional configuration fields for extensibility
-    /// This allows forward-compatibility with future fields like catalog_type, catalog_properties
-    #[serde(flatten)]
-    pub extra: HashMap<String, toml::Value>,
+/// Engine configuration as a tagged enum
+#[derive(Debug, Clone, Deserialize)]
+#[serde(tag = "type", rename_all = "lowercase")]
+pub enum EngineConfig {
+    Datafusion {
+        #[serde(default)]
+        catalog: Option<CatalogConfig>,
+    },
 }
 
 #[async_trait::async_trait]
 pub trait EngineRunner: Send {
     async fn run_slt_file(&mut self, path: &Path) -> Result<()>;
 }
 
-pub async fn load_engine_runner(name: &str, config: EngineConfig) -> Result<Box<dyn EngineRunner>> {
-    match config.engine_type {
-        EngineType::Datafusion => Ok(Box::new(DataFusionEngine::new(name, &config).await?)),
+pub async fn load_engine_runner(config: EngineConfig) -> Result<Box<dyn EngineRunner>> {
+    match config {
+        EngineConfig::Datafusion { catalog } => {
+            Ok(Box::new(DataFusionEngine::new(catalog).await?))
+        }
     }
 }
 
@@ -80,45 +83,63 @@ where
 
 #[cfg(test)]
 mod tests {
-    use std::collections::HashMap;
-
-    use crate::engine::{EngineConfig, EngineType, load_engine_runner};
+    use crate::engine::{CatalogConfig, EngineConfig, load_engine_runner};
 
     #[test]
     fn test_deserialize_engine_config() {
-        let input = r#"
-            type = "datafusion"
-        "#;
+        let input = r#"type = "datafusion""#;
 
         let config: EngineConfig = toml::from_str(input).unwrap();
-        assert_eq!(config.engine_type, EngineType::Datafusion);
-        assert!(config.extra.is_empty());
+        assert!(matches!(config, EngineConfig::Datafusion { catalog: None }));
     }
 
     #[test]
-    fn test_deserialize_engine_config_with_extras() {
+    fn test_deserialize_engine_config_with_catalog() {
         let input = r#"
             type = "datafusion"
-            catalog_type = "rest"
 
-            [catalog_properties]
+            [catalog]
+            type = "rest"
+
+            [catalog.props]
             uri = "http://localhost:8181"
         "#;
 
         let config: EngineConfig = toml::from_str(input).unwrap();
-        assert_eq!(config.engine_type, EngineType::Datafusion);
-        assert!(config.extra.contains_key("catalog_type"));
-        assert!(config.extra.contains_key("catalog_properties"));
+        match config {
+            EngineConfig::Datafusion { catalog: Some(cat) } => {
+                assert_eq!(cat.catalog_type, "rest");
+                assert_eq!(
+                    cat.props.get("uri"),
+                    Some(&"http://localhost:8181".to_string())
+                );
+            }
+            _ => panic!("Expected Datafusion with catalog"),
+        }
+    }
+
+    #[test]
+    fn test_deserialize_catalog_config() {
+        let input = r#"
+            type = "memory"
+
+            [props]
+            warehouse = "file:///tmp/warehouse"
+        "#;
+
+        let config: CatalogConfig = toml::from_str(input).unwrap();
+        assert_eq!(config.catalog_type, "memory");
+        assert_eq!(
+            config.props.get("warehouse"),
+            Some(&"file:///tmp/warehouse".to_string())
+        );
     }
 
     #[tokio::test]
     async fn test_load_datafusion() {
-        let config = EngineConfig {
-            engine_type: EngineType::Datafusion,
-            extra: HashMap::new(),
-        };
+        let config = EngineConfig::Datafusion { catalog: None };
 
-        let result = load_engine_runner("df", config).await;
+        let result = load_engine_runner(config).await;
         assert!(result.is_ok());
     }
 }

crates/sqllogictest/src/schedule.rs

@@ -83,7 +83,7 @@ impl Schedule {
         let mut engines = HashMap::new();
 
         for (name, config) in configs {
-            let engine = load_engine_runner(&name, config).await?;
+            let engine = load_engine_runner(config).await?;
             engines.insert(name, engine);
         }
 
@@ -129,7 +129,7 @@ impl Schedule {
 
 #[cfg(test)]
 mod tests {
-    use crate::engine::EngineType;
+    use crate::engine::EngineConfig;
     use crate::schedule::ScheduleConfig;
 
     #[test]
@@ -147,7 +147,10 @@ mod tests {
 
         assert_eq!(config.engines.len(), 1);
         assert!(config.engines.contains_key("df"));
-        assert_eq!(config.engines["df"].engine_type, EngineType::Datafusion);
+        assert!(matches!(
+            config.engines["df"],
+            EngineConfig::Datafusion { catalog: None }
+        ));
         assert_eq!(config.steps.len(), 1);
         assert_eq!(config.steps[0].engine, "df");
         assert_eq!(config.steps[0].slt, "test.slt");
@@ -178,11 +181,16 @@ mod tests {
     }
 
     #[test]
-    fn test_deserialize_with_extra_fields() {
-        // Test forward-compatibility with extra fields (for PR #1943)
+    fn test_deserialize_with_catalog_config() {
         let input = r#"
-            [engines]
-            df = { type = "datafusion", catalog_type = "rest", some_future_field = "value" }
+            [engines.df]
+            type = "datafusion"
+
+            [engines.df.catalog]
+            type = "rest"
+
+            [engines.df.catalog.props]
+            uri = "http://localhost:8181"
 
             [[steps]]
             engine = "df"
@@ -191,8 +199,16 @@ mod tests {
 
         let config: ScheduleConfig = toml::from_str(input).unwrap();
 
-        assert!(config.engines["df"].extra.contains_key("catalog_type"));
-        assert!(config.engines["df"].extra.contains_key("some_future_field"));
+        match &config.engines["df"] {
+            EngineConfig::Datafusion { catalog: Some(cat) } => {
+                assert_eq!(cat.catalog_type, "rest");
+                assert_eq!(
+                    cat.props.get("uri"),
+                    Some(&"http://localhost:8181".to_string())
+                );
+            }
+            _ => panic!("Expected Datafusion with catalog config"),
+        }
     }
 
     #[test]