CTTY commented on code in PR #1953:
URL: https://github.com/apache/iceberg-rust/pull/1953#discussion_r2641456296
##########
crates/sqllogictest/src/engine/datafusion.rs:
##########
@@ -59,20 +58,22 @@ impl EngineRunner for DataFusionEngine {
}
impl DataFusionEngine {
- pub async fn new(config: TomlTable) -> Result<Self> {
+ pub async fn new(_name: &str, config: &EngineConfig) -> Result<Self> {
Review Comment:
Could you help me understand why do we add the `name` here? I can't think of
any cases where `name` would matter
##########
crates/sqllogictest/src/engine/mod.rs:
##########
@@ -17,29 +17,44 @@
mod datafusion;
+use std::collections::HashMap;
use std::path::Path;
use anyhow::anyhow;
+use serde::Deserialize;
use sqllogictest::{AsyncDB, MakeConnection, Runner, parse_file};
-use toml::Table as TomlTable;
use crate::engine::datafusion::DataFusionEngine;
use crate::error::{Error, Result};
-const TYPE_DATAFUSION: &str = "datafusion";
+/// 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
+#[derive(Debug, Clone, Deserialize)]
+pub struct EngineConfig {
+ /// The type of engine
+ #[serde(rename = "type")]
+ pub engine_type: EngineType,
+
+ /// 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>,
Review Comment:
This is an interesting direction, but I'm feeling that we will need another
serializable config like CatalogConfig in the future rather than using
toml::Value directly
--
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]
