jorgecarleitao commented on a change in pull request #8080:
URL: https://github.com/apache/arrow/pull/8080#discussion_r479785852
##########
File path: rust/datafusion/src/sql/planner.rs
##########
@@ -592,7 +607,7 @@ mod tests {
fn select_scalar_func_with_literal_no_relation() {
quick_test(
"SELECT sqrt(9)",
- "Projection: sqrt(CAST(Int64(9) AS Float64))\
+ "Projection: sqrt(Int64(9))\
Review comment:
It depends where want to perform the cast. This test was ensuring that
the cast was happenning on the logical plane, due to the type coercer.
IMO `sqrt(8)` is a logical expression that everyone agrees should return
`2.82842712475...` and the reason we cast it to f64 is entirely due to a
limitation of our physical expressions, that only implement `sqrt` for f64.
For this reason, IMO these type of casting operations should only be done on
our physical plane. For example, a different physical planner could prefer to
not have it `cast`ed because it has a specialized implementation that supports
other types.
This PR moves casting operations on built-in functions to the physical
plane, like we are performing for binary operators.
Note that the user continues to have full transparency: they can use
`EXPLAIN verbose` to get the physical plan, that contains the cast.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
