iffyio commented on code in PR #1747:
URL: 
https://github.com/apache/datafusion-sqlparser-rs/pull/1747#discussion_r1988442245


##########
src/ast/dml.rs:
##########
@@ -138,6 +143,30 @@ pub struct CreateTable {
     pub engine: Option<TableEngine>,
     pub comment: Option<CommentDef>,
     pub auto_increment_offset: Option<u32>,
+    pub key_block_size: Option<u32>,

Review Comment:
   > Add another parameter Vec which will include properties which do not 
require a preceding keyword (WITH/OPTIONS/...), e.g. plain_options
   
   seeing as there's a few similar parameters, thinking it could be nice to 
merge the variants into an enum something like
   ```rust
   enum CreateTableOptions {
     TableProperties(Vec<SqlOption>),
     WithOptions(Vec<SqlOption>),
     Options(Vec<SqlOption>),
     Plain(Vec<SqlOption>),
   }
   ```
   that doesn't have to be in this PR however, we could introduce a new field 
if it could be a hassle to merge the existing ones.
   
   
   > Parameter parsing will be delegated to the respective Dialect to avoid 
clutter in the main flow
   
   How do we mean by this part? I imagined it would be reusing 
`self.parse_options`



-- 
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