alamb commented on issue #10256:
URL: https://github.com/apache/datafusion/issues/10256#issuecomment-2085317765

   > Yeah, so if we encountered a SortExpr in expr_to_sql function, does that 
mean unexpected location ? I think for a entire query case, the code will 
probably never reach that.
   
   I think so
   
   > But `col("a").sort(true, true)` will be only "a", which doesn't match the 
sql semantic? 🤔
   
   Creating SQL from programatically created `Expr`s is an interesting usecase.
   
   it seems like the issue is that `Expr` in datafusion can come from both 
`ast::Expr` and 
[`ast::OrderByExpr`](https://docs.rs/sqlparser/latest/sqlparser/ast/struct.OrderByExpr.html)
   
   Currently we have 
   
https://github.com/apache/datafusion/blob/dd5683745e7d527b01b804c8f4f1a0a53aa225e8/datafusion/sql/src/unparser/expr.rs#L47-L50
   
   Maybe we could change the signature to something like 
   ```rust
   /// DataFusion's Exprs can represent either an `Expr` or an `OrderByExpr`
   enum Unparsed {
     // SQL Expression
     Expr(ast::Expr),
     // SQL ORDER BY expression (e.g. `col ASC NULLS FIRST`)
     OrderByExpr(ast::OrderByExpr),
   }
    
   pub fn expr_to_sql(expr: &Expr) -> Result<Unparsed> { 
   ...
    } 
   ```
   
   
   


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