-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support coercing utf8
to interval
and timestamp
(including arguments to date_bin
)
#5117
Conversation
f06ad3c
to
b54dc5a
Compare
fn cast_expr(expr: &Expr, to_type: &DataType, schema: &DFSchema) -> Result<Expr> { | ||
// Special case until Interval coercion is handled in arrow-rs | ||
// https://github.com/apache/arrow-rs/issues/3643 | ||
match (expr, to_type) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only supports literals at the moment -- tracked apache/arrow-rs#3643 for the more general case of coercing columns
f310f41
to
7497cc4
Compare
utf8
to interval
and timestamp
(including arguments to date_bin)utf8
to interval
and timestamp
(including arguments to date_bin
)
cc @waitingkuo and @comphead |
@@ -76,6 +76,18 @@ SELECT DATE_BIN(INTERVAL '15 minutes', TIMESTAMP '2022-08-03 14:38:50Z', TIMESTA | |||
---- | |||
2022-08-03T14:30:00 | |||
|
|||
# Can coerce string interval arguments | |||
query T | |||
SELECT DATE_BIN('15 minutes', TIMESTAMP '2022-08-03 14:38:50Z', TIMESTAMP '1970-01-01T00:00:00Z') AS res |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small nit: if AS res is needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you are right -- res
is not needed. I will remove them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In 7a7fd4b
LGTM, not permitted to approve the PR :) |
utf8
to interval
and timestamp
(including arguments to date_bin
)utf8
to interval
and timestamp
(including arguments to date_bin
)
Benchmark runs are scheduled for baseline = 7d2d51b and contender = 224c682. 224c682 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Draft as itWhich issue does this PR close?
Closes #4853
Rationale for this change
Rationale:
date_bin
with a string value does not work and gives a hard to understand message:What changes are included in this PR?
Utf8
constants can be automatically coerced into Intervals (same as today)Utf8
can be automatically coerced into TimestampsAre these changes tested?
Yes
Are there any user-facing changes?
Yes, strings are now automatically coerced into
Timetamp
s andIntervals
Note this behavior is consistent with Postgres: