devinjdangelo commented on issue #8661:
URL: 
https://github.com/apache/arrow-datafusion/issues/8661#issuecomment-1991665592

   > Does it make sense for me to lay the same 'groundwork' for the LogicalPlan 
serialization as I did for the Expr in 
https://github.com/apache/arrow-datafusion/pull/9517?
   
   I am in support of this. We should be cautious about what we expose via 
public interfaces at first, since I think it is likely we will require many 
breaking changes to support all possible logical plans. The top level 
`plan_to_sql` function should be stable, but we will need freedom to rework 
methods below that. 
   
   > Upstream the [AST 
builders](https://github.com/datafusion-contrib/datafusion-federation/blob/35459193fd66c4476fceae65df5f3163242789c7/sql-writer/src/ast_builder.rs)
 to sqlparser-rs.
   
   I am a bit surprised there isn't an ergonomic way to build an AST already in 
`sqlparser`. I can see that the DFParser constructs Statements directly without 
a builder struct. 
   
   If the AST builder functionality is not particularly useful in `sqlparser`, 
it is likely best to include that in datafusion only rather than upstream to 
`sqlparser`. Do you think the AST builders would be useful upstream @alamb?
   


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

Reply via email to