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