andygrove commented on code in PR #3763:
URL: https://github.com/apache/arrow-datafusion/pull/3763#discussion_r992828969
##########
datafusion/sql/src/planner.rs:
##########
@@ -5309,6 +5456,19 @@ mod tests {
assert!(logical_plan("SELECT \"1\"").is_err());
}
+ #[test]
+ fn test_datetime_keyword_conversion() {
+ // Field "year" does not exist in table.
+ let sql_expr = "SELECT AVG(year) FROM person";
+ let err1 = logical_plan(sql_expr).expect_err("query should have
failed");
+ assert_plan_error(err1, "Avg");
Review Comment:
> The general idea of this test is that originally, the query would have
failed with a "Field Not Found" error because "year" isn't in the "person"
table.
That sounds like the correct behavior to me. Here is an example from
Postgres:
```
postgres=# select avg(year) from person;
ERROR: column "year" does not exist
LINE 1: select avg(year) from person;
```
> However, because Avg is a function and "year" isn't in the table, we want
to convert "year" to a datetime keyword to check if it is valid, so with my
changes it fails with a "The function Avg does not support inputs of type Utf8"
instead.
I think it would be useful to have a test of a valid use of year as a
datetime keyword, as I am not fully understanding the goal here yet.
> This is a pretty roundabout way of testing the functionality, but because
these changes were implemented to allow datetime keyword conversion before
calling something like Timestampadd in Dask-SQL, which DataFusion does not
support, this was the alternative testing method I came up with. Let me know
what you think.
I will ping you directly with questions about the Dask SQL requirement
driving this.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
