[CALCITE-5508] Allow no-op versions of DATETIME and TIMESTAMP #3521
Conversation
|
Kudos, SonarCloud Quality Gate passed!
|
| @@ -5055,4 +5055,24 @@ void checkUserDefinedOrderByOver(NullCollation nullCollation) { | |||
| String sql = "SELECT CAST(CAST(? AS INTEGER) AS CHAR)"; | |||
| sql(sql).ok(); | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
there is no documentation added in this PR for these behaviors.
Also, it would be nice to have some negative tests, where type inference rejects the query, and with NULL arguments.
There was a problem hiding this comment.
I'll try to come up with a negative test shortly. In the meantime, I've added documentation for the new overloads, and made some minor corrections to the existing documentation so they more accurately reflect how BigQuery actually handles these functions.
The no-op versions are undocumented by BigQuery, but easily verifiable as working with a simple example query: SELECT DATETIME(DATETIME '2022-02-02 12:12:12')
| @@ -2708,6 +2708,18 @@ SELECT TIMESTAMP(DATE "2008-12-25") AS timestamp_date; | |||
|
|
|||
| !ok | |||
|
|
|||
| # This tests the no-op TIMESTAMP and DATETIME functions. | |||
| select TIMESTAMP(TIMESTAMP "2008-01-01 01:03:05") as ts, | |||
There was a problem hiding this comment.
Maybe using !type would be helpful here since the aim is to show they return the same type as they are given.
There was a problem hiding this comment.
or use !ok and !type in conjunction somehow
| @@ -2833,12 +2835,13 @@ BigQuery's type system uses confusingly different names for types and functions: | |||
| | b | TIME(timestamp) | Extracts the TIME from *timestamp* (a local time; BigQuery's DATETIME type) | |||
| | b | TIME(instant) | Extracts the TIME from *timestampLtz* (an instant; BigQuery's TIMESTAMP type), assuming UTC | |||
| | b | TIME(instant, timeZone) | Extracts the time from *timestampLtz* (an instant; BigQuery's TIMESTAMP type), in *timeZone* | |||
| | b | TIMESTAMP(timestampLtz) | Returns its argument unchanged (no-op) | |||
There was a problem hiding this comment.
would returns *timestampLtz* unchanged (no-op) fit better with other descriptions?
| @@ -2711,9 +2711,11 @@ BigQuery's type system uses confusingly different names for types and functions: | |||
| | p q | DATEADD(timeUnit, integer, datetime) | Equivalent to `TIMESTAMPADD(timeUnit, integer, datetime)` | |||
| | p q | DATEDIFF(timeUnit, datetime, datetime2) | Equivalent to `TIMESTAMPDIFF(timeUnit, datetime, datetime2)` | |||
| | q | DATEPART(timeUnit, datetime) | Equivalent to `EXTRACT(timeUnit FROM datetime)` | |||
| | b | DATETIME(timestamp) | Returns its argument unchanged (no-op) | |||
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions. |
This is a bit of unfinished work from CALCITE-5508. BigQuery supports calling the
TIMESTAMP()function with aTIMESTAMP-typed argument, and theDATETIME()function with aDATETIME-typed argument. Both functions should simply return their argument in these cases, which is functionality we get for free from the existing implementations.The problem is that, without proper argument type checking, Calcite will try to coerce the argument types, which in the case of
DATETIME()causes it to try casting asDATEand eliminating the time component.