yyy1000 commented on issue #10256: URL: https://github.com/apache/datafusion/issues/10256#issuecomment-2080708518
Thank you all for your reply! > when turning the expression back into an entire query Yeah, I was just a little curious why for Expr::Sort we ignore the ORDER BY information. It seems that for the entire query, it could address the Sort. https://github.com/apache/datafusion/blob/f8c623fe045d70a87eac8dc8620b74ff73be56d5/datafusion/sql/src/unparser/plan.rs#L222-L231 > Maybe the code would be clearer if we made the conversion code return an Error if it encountered a SortExpr in an unexpected location 🤔 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 know a use case would be just convert a `Expr` to sql. But `col("a").sort(true, true)` will be only "a", which doesn't match the sql semantic? 🤔 > If I understand correctly, I think another case is Expr::AggregateFunction, where we can't really convert it back to [ast::Function] (https://docs.rs/sqlparser/latest/sqlparser/ast/struct.Function.html#) as it's also not an enum item in ast::Expr. ast::Function is an enum item in ast::Expr, see https://github.com/sqlparser-rs/sqlparser-rs/blob/0b5722afbfb60c3e3a354bc7c7dc02cb7803b807/src/ast/mod.rs#L683-L684 So maybe the return type of expr_to_sql being ast::Expr is enough, given the `Sort` will be addressed in a special way? -- 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
