SQL: Improved behavior when implicitly casting strings to date/time literals.#5023
SQL: Improved behavior when implicitly casting strings to date/time literals.#5023jihoonson merged 2 commits intoapache:masterfrom
Conversation
gianm
commented
Oct 31, 2017
gianm
commented
- Handle all flavors of ISO8601 and SQL literals.
- Throw errors on other literals instead of silently transforming them to 0.
…iterals. - Handle all flavors of ISO8601 and SQL literals. - Throw errors on other literals instead of silently transforming them to 0.
|
👍 |
| ImmutableList.of( | ||
| operand, | ||
| DruidExpression.fromExpression(DruidExpression.stringLiteral(dateTimeFormatString(toType))), | ||
| DruidExpression.fromExpression(DruidExpression.nullLiteral()), |
There was a problem hiding this comment.
Does this mean the dateTimeFormat parameter is always null when casting? If so, the below timeZone parameter looks not necessary.
There was a problem hiding this comment.
It does, but the timezone is still useful, since null for the format really just means "use iso or sql format" and that may still need a time zone to interpret the value.
For example timestamp_parse('2000-01-01 00:00:00', null, 'UTC') and timestamp_parse('2000-01-01 00:00:00', null, 'America/Los_Angeles') return different values.
There was a problem hiding this comment.
I mean, timezone looks not used if formatString is null (https://github.com/druid-io/druid/blob/master/processing/src/main/java/io/druid/query/expression/TimestampParseExprMacro.java#L58-L61). Maybe we need to fix TimestampParseExprMacro to use timezone for null formatString as well?
There was a problem hiding this comment.
Oh, sorry, you're right! This is a bug in TimestampParseExprMacro. I pushed another commit fixing it to this PR, and added a test.
3 participants
