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