Skip to content

Conversation

@waitingkuo
Copy link
Contributor

… other units; add test cases

Which issue does this PR close?

Closes #2998

Rationale for this change

to make this works

select to_timestamp_seconds(a) from (select to_timestamp_seconds(1) as a)A;
+-------------------------+
| totimestampseconds(a.a) |
+-------------------------+
| 1970-01-01 00:00:01     |
+-------------------------+
1 row in set. Query took 0.004 seconds.
❯ select to_timestamp_seconds(a) from (select to_timestamp_seconds(1) as a)A;

the subquery select to_timestamp_seconds(1) as a output a as TimeUnit::Second. However it's not in to_timestamp_seconds its own signatures, so it's been converted to utf-8 1970-01-01T00:00:01 as utf-8 listed in the first signature. While there's no explicit timezone, it will be deemed as local time, that will cause the second to_timestamp_seconds produce the timestamp with timezone offset. https://docs.rs/arrow/19.0.0/arrow/compute/kernels/cast_utils/fn.string_to_timestamp_nanos.html#timezone--offset-handling

This pr is to add TimeUnit::Second as signature for ToTimestampSecond

similar as other units:

select to_timestamp(a) from (select to_timestamp(1) as a)A;
+-------------------------------+
| totimestamp(a.a)              |
+-------------------------------+
| 1970-01-01 00:00:00.000000001 |
+-------------------------------+
1 row in set. Query took 0.053 seconds.
select to_timestamp_millis(a) from (select to_timestamp_millis(1) as a)A;
+-------------------------+
| totimestampmillis(a.a)  |
+-------------------------+
| 1970-01-01 00:00:00.001 |
+-------------------------+
1 row in set. Query took 0.003 seconds.
select to_timestamp_micros(a) from (select to_timestamp_micros(1) as a)A;
+----------------------------+
| totimestampmicros(a.a)     |
+----------------------------+
| 1970-01-01 00:00:00.000001 |
+----------------------------+
1 row in set. Query took 0.003 seconds.

What changes are included in this PR?

add TimeUnit::Second as signature for ToTimestampSecond
add TimeUnit::Millisecond as signature for ToTimestampMillis
add TimeUnit::Microsecond as signature for ToTimestampMicros
add TimeUnit::Nanosecond as signature for ToTimestamp

Are there any user-facing changes?

No

@github-actions github-actions bot added core Core DataFusion crate logical-expr Logical plan and expressions labels Aug 1, 2022
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM -- thank you @waitingkuo

@alamb alamb merged commit 55a1286 into apache:master Aug 1, 2022
@ursabot
Copy link

ursabot commented Aug 1, 2022

Benchmark runs are scheduled for baseline = c179102 and contender = 55a1286. 55a1286 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] test-mac-arm
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] ursa-i9-9960x
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate logical-expr Logical plan and expressions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Double to_timestamp_seconds produces abnormal result

3 participants