findepi commented on code in PR #13517:
URL: https://github.com/apache/datafusion/pull/13517#discussion_r1855189329


##########
datafusion/expr/src/logical_plan/ddl.rs:
##########
@@ -288,8 +290,234 @@ impl PartialOrd for CreateExternalTable {
     }
 }
 
+impl CreateExternalTable {
+    pub fn new(fields: CreateExternalTableFields) -> Result<Self> {
+        let CreateExternalTableFields {
+            name,
+            schema,
+            location,
+            file_type,
+            table_partition_cols,
+            if_not_exists,
+            temporary,
+            definition,
+            order_exprs,
+            unbounded,
+            options,
+            constraints,
+            column_defaults,
+        } = fields;
+        check_fields_unique(&schema)?;
+        Ok(Self {
+            name,
+            schema,
+            location,
+            file_type,
+            table_partition_cols,
+            if_not_exists,
+            temporary,
+            definition,
+            order_exprs,
+            unbounded,
+            options,
+            constraints,
+            column_defaults,
+        })
+    }
+
+    pub fn into_fields(self) -> CreateExternalTableFields {
+        let Self {
+            name,
+            schema,
+            location,
+            file_type,
+            table_partition_cols,
+            if_not_exists,
+            temporary,
+            definition,
+            order_exprs,
+            unbounded,
+            options,
+            constraints,
+            column_defaults,
+        } = self;
+        CreateExternalTableFields {
+            name,
+            schema,
+            location,
+            file_type,
+            table_partition_cols,
+            if_not_exists,
+            temporary,
+            definition,
+            order_exprs,
+            unbounded,
+            options,
+            constraints,
+            column_defaults,
+        }
+    }
+
+    pub fn builder() -> CreateExternalTableBuilder {
+        CreateExternalTableBuilder::new()
+    }
+}
+
+/// A struct with same fields as [`CreateExternalTable`] struct so that the 
DDL can be conveniently
+/// destructed with validation that each field is handled, while still 
requiring that all
+/// construction goes through the [`CreateExternalTable::new`] constructor or 
the builder.
+pub struct CreateExternalTableFields {
+    /// The table name
+    pub name: TableReference,
+    /// The table schema
+    pub schema: DFSchemaRef,
+    /// The physical location
+    pub location: String,
+    /// The file type of physical file
+    pub file_type: String,
+    /// Partition Columns
+    pub table_partition_cols: Vec<String>,
+    /// Option to not error if table already exists
+    pub if_not_exists: bool,
+    /// Whether the table is a temporary table
+    pub temporary: bool,
+    /// SQL used to create the table, if available
+    pub definition: Option<String>,
+    /// Order expressions supplied by user
+    pub order_exprs: Vec<Vec<Sort>>,
+    /// Whether the table is an infinite streams
+    pub unbounded: bool,
+    /// Table(provider) specific options
+    pub options: HashMap<String, String>,
+    /// The list of constraints in the schema, such as primary key, unique, 
etc.
+    pub constraints: Constraints,
+    /// Default values for columns
+    pub column_defaults: HashMap<String, Expr>,
+}
+
+/// A builder or [`CreateExternalTable`]. Use [`CreateExternalTable::builder`] 
to obtain a new builder instance.
+pub struct CreateExternalTableBuilder {
+    name: Option<TableReference>,
+    schema: Option<DFSchemaRef>,
+    location: Option<String>,
+    file_type: Option<String>,
+    table_partition_cols: Vec<String>,
+    if_not_exists: bool,
+    temporary: bool,
+    definition: Option<String>,
+    order_exprs: Vec<Vec<Sort>>,
+    unbounded: bool,
+    options: HashMap<String, String>,
+    constraints: Constraints,
+    column_defaults: HashMap<String, Expr>,
+}
+
+impl CreateExternalTableBuilder {
+    fn new() -> Self {
+        Self {
+            name: None,
+            schema: None,
+            location: None,
+            file_type: None,
+            table_partition_cols: vec![],
+            if_not_exists: false,
+            temporary: false,
+            definition: None,
+            order_exprs: vec![],
+            unbounded: false,
+            options: HashMap::new(),
+            constraints: Constraints::empty(),
+            column_defaults: HashMap::new(),
+        }
+    }
+
+    pub fn name(mut self, name: TableReference) -> Self {
+        self.name = Some(name);
+        self
+    }
+
+    pub fn schema(mut self, schema: DFSchemaRef) -> Self {
+        self.schema = Some(schema);
+        self
+    }
+
+    pub fn location(mut self, location: String) -> Self {
+        self.location = Some(location);
+        self
+    }
+
+    pub fn file_type(mut self, file_type: String) -> Self {
+        self.file_type = Some(file_type);
+        self
+    }
+
+    pub fn table_partition_cols(mut self, table_partition_cols: Vec<String>) 
-> Self {
+        self.table_partition_cols = table_partition_cols;
+        self
+    }
+
+    pub fn if_not_exists(mut self, if_not_exists: bool) -> Self {
+        self.if_not_exists = if_not_exists;
+        self
+    }
+
+    pub fn temporary(mut self, temporary: bool) -> Self {
+        self.temporary = temporary;
+        self
+    }
+
+    pub fn definition(mut self, definition: Option<String>) -> Self {
+        self.definition = definition;
+        self
+    }
+
+    pub fn order_exprs(mut self, order_exprs: Vec<Vec<Sort>>) -> Self {
+        self.order_exprs = order_exprs;
+        self
+    }
+
+    pub fn unbounded(mut self, unbounded: bool) -> Self {
+        self.unbounded = unbounded;
+        self
+    }
+
+    pub fn options(mut self, options: HashMap<String, String>) -> Self {
+        self.options = options;
+        self
+    }
+
+    pub fn constraints(mut self, constraints: Constraints) -> Self {
+        self.constraints = constraints;
+        self
+    }
+
+    pub fn column_defaults(mut self, column_defaults: HashMap<String, Expr>) 
-> Self {
+        self.column_defaults = column_defaults;
+        self
+    }
+
+    pub fn build(self) -> Result<CreateExternalTable> {
+        CreateExternalTable::new(CreateExternalTableFields {
+            name: self.name.expect("name is required"),
+            schema: self.schema.expect("schema is required"),
+            location: self.location.expect("location is required"),
+            file_type: self.file_type.expect("file_type is required"),
+            table_partition_cols: self.table_partition_cols,
+            if_not_exists: self.if_not_exists,
+            temporary: self.temporary,
+            definition: self.definition,
+            order_exprs: self.order_exprs,
+            unbounded: self.unbounded,
+            options: self.options,
+            constraints: self.constraints,
+            column_defaults: self.column_defaults,
+        })
+    }
+}
+
 /// Creates an in memory table.
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Hash)]
+#[non_exhaustive]

Review Comment:
   To disallow public direct construction. If we allow public direct 
construction, we can't enforce invariants in the factory method, since it may 
or may not get called.
   
   



-- 
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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to