alamb commented on issue #10256: URL: https://github.com/apache/datafusion/issues/10256#issuecomment-2080448399
I don't think we should change SQL parser In general, `ORDER BY ...` isn't really a general Expr (it can't appear in arbitrary locations in SQL, only specific ones) For example, this isn't valid; ```sql SELECT x ORDER BY y FROM foo; ``` ORDER BY can appear in certain places like ```sql SELECT x FROM foo ORDER BY y SELECT agg(x ORDER BY y) FROM foo; SELECT agg(..) OVER (ORDER BY y) FROM foo ``` I think we should special case finding `Expr::Sort` exprs in those locations. Maybe the code would be clearer if we made the conversion code return an Error if it encountered a SortExpr in an unexpected location 🤔 -- 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
