findepi commented on code in PR #13489:
URL: https://github.com/apache/datafusion/pull/13489#discussion_r1849941066
##########
datafusion/expr/src/logical_plan/ddl.rs:
##########
@@ -285,8 +288,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 {
Review Comment:
Yes, it does introduce code duplication and so does the builder.
When handling the Create Table/View, deconstruction is valuable without
`..`, as it guarantees that any new field will force the code to be revisited
(rather than new fields being ignored).
However, in Rust deconstruction without `..` is possible only when
construction is possible, and direct construction being possible precludes
construction-time checks, which is undesirable.
Alternatively to this, we could allow construction of ill-formed Create
Table/View objects, and have check somewhere else (plan validator), but i would
be worried that such a delayed check could be missed in some code flows. The
field duplication isn't a problem from maintainability perspective after all.
--
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]
