liurenjie1024 commented on code in PR #1666:
URL: https://github.com/apache/iceberg-rust/pull/1666#discussion_r2375200111
##########
crates/catalog/sql/src/catalog.rs:
##########
@@ -83,7 +208,7 @@ pub struct SqlCatalog {
sql_bind_style: SqlBindStyle,
}
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, strum::EnumString, strum::Display, PartialOrd, Eq)]
Review Comment:
We also need to derive `Ord`?
##########
crates/catalog/sql/src/catalog.rs:
##########
@@ -54,6 +61,125 @@ static MAX_CONNECTIONS: u32 = 10; // Default the SQL pool
to 10 connections if n
static IDLE_TIMEOUT: u64 = 10; // Default the maximum idle timeout per
connection to 10s before it is closed
static TEST_BEFORE_ACQUIRE: bool = true; // Default the health-check of each
connection to enabled prior to returning
+/// Builder for [`SqlCatalog`]
+#[derive(Debug)]
+pub struct SqlCatalogBuilder(SqlCatalogConfig);
+
+impl Default for SqlCatalogBuilder {
+ fn default() -> Self {
+ Self(SqlCatalogConfig {
+ uri: "".to_string(),
+ name: "".to_string(),
+ warehouse_location: "".to_string(),
+ file_io: FileIOBuilder::new_fs_io().build().unwrap(),
+ sql_bind_style: SqlBindStyle::DollarNumeric,
+ props: HashMap::new(),
+ })
+ }
+}
+
+impl SqlCatalogBuilder {
+ /// Configure the database URI
+ ///
+ /// If `SQL_CATALOG_PROP_URI` has a value set in `props` during
`SqlCatalogBuilder::load`,
+ /// that value takes precedence, and the value specified by this method
will not be used.
+ pub fn uri(mut self, uri: impl Into<String>) -> Self {
+ self.0.uri = uri.into();
+ self
+ }
+
+ /// Configure the warehouse location
+ ///
+ /// If `SQL_CATALOG_PROP_WAREHOUSE` has a value set in `props` during
`SqlCatalogBuilder::load`,
+ /// that value takes precedence, and the value specified by this method
will not be used.
+ pub fn warehouse_location(mut self, location: impl Into<String>) -> Self {
+ self.0.warehouse_location = location.into();
+ self
+ }
+
+ /// Configure the bound SQL Statement
+ ///
+ /// If `SQL_CATALOG_PROP_BIND_STYLE` has a value set in `props` during
`SqlCatalogBuilder::load`,
+ /// that value takes precedence, and the value specified by this method
will not be used.
+ pub fn sql_bind_style(mut self, sql_bind_style: SqlBindStyle) -> Self {
+ self.0.sql_bind_style = sql_bind_style;
+ self
+ }
+
+ /// Configure the any properties
+ ///
+ /// If the same key has values set in `props` during
`SqlCatalogBuilder::load`,
+ /// those values will take precedence.
+ pub fn props(mut self, props: HashMap<String, String>) -> Self {
+ self.0.props = props;
Review Comment:
I don't think we should simply replace the map, we should merge with
existing map.
##########
crates/catalog/sql/src/catalog.rs:
##########
@@ -54,6 +61,125 @@ static MAX_CONNECTIONS: u32 = 10; // Default the SQL pool
to 10 connections if n
static IDLE_TIMEOUT: u64 = 10; // Default the maximum idle timeout per
connection to 10s before it is closed
static TEST_BEFORE_ACQUIRE: bool = true; // Default the health-check of each
connection to enabled prior to returning
+/// Builder for [`SqlCatalog`]
+#[derive(Debug)]
+pub struct SqlCatalogBuilder(SqlCatalogConfig);
+
+impl Default for SqlCatalogBuilder {
+ fn default() -> Self {
+ Self(SqlCatalogConfig {
+ uri: "".to_string(),
+ name: "".to_string(),
+ warehouse_location: "".to_string(),
+ file_io: FileIOBuilder::new_fs_io().build().unwrap(),
Review Comment:
We should not panic here, instead we should report an error.
--
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]
