`date_part` support fractions of second #4385

comphead · 2022-11-27T01:39:02Z

Which issue does this PR close?

Closes #3997

Rationale for this change

See #3997

What changes are included in this PR?

Extending date_part to support millis, micros, nanos

Are these changes tested?

Yes

Are there any user-facing changes?

No

comphead · 2022-11-27T01:41:23Z

@waitingkuo finally we can get back to #3997 resolution.
Please check the PR.

Now date_part supports second fraction, but its still not the same as in PSQL like

willy=# select date_part('second', timestamp '2000-01-01T00:00:00.1');
 date_part 
-----------
       0.1
(1 row)

To do the same you need to run

select date_part('milliseconds', timestamp)
 date_part 
-----------
       1
(1 row)

Let me know if this is ok

waitingkuo · 2022-11-28T22:25:01Z

hi @comphead thank you

i tested the pr

❯ select date_part('second', timestamp '2000-01-01T00:00:09.123456');
+-------------------------------------------------------------+
| datepart(Utf8("second"),Utf8("2000-01-01T00:00:09.123456")) |
+-------------------------------------------------------------+
| 9                                                           |
+-------------------------------------------------------------+
1 row in set. Query took 0.002 seconds.
❯ select date_part('millisecond', timestamp '2000-01-01T00:00:09.123456');
+------------------------------------------------------------------+
| datepart(Utf8("millisecond"),Utf8("2000-01-01T00:00:09.123456")) |
+------------------------------------------------------------------+
| 123                                                              |
+------------------------------------------------------------------+
1 row in set. Query took 0.002 seconds.
❯ select date_part('microsecond', timestamp '2000-01-01T00:00:09.123456');
+------------------------------------------------------------------+
| datepart(Utf8("microsecond"),Utf8("2000-01-01T00:00:09.123456")) |
+------------------------------------------------------------------+
| 123456                                                           |
+------------------------------------------------------------------+
1 row in set. Query took 0.002 seconds.

the behavior is different than what postgresql has

willy=# select date_part('second', timestamp '2000-01-01T00:00:09.123456');
 date_part 
-----------
  9.123456
(1 row)

willy=# select date_part('millisecond', timestamp '2000-01-01T00:00:09.123456');
 date_part 
-----------
  9123.456
(1 row)

willy=# select date_part('microsecond', timestamp '2000-01-01T00:00:09.123456');
 date_part 
-----------
   9123456
(1 row)

i checked some other system,
spark seems to have the same behavior as postgresql

# this is spark
SELECT date_part('SECONDS', timestamp'2019-10-01 00:00:01.000001');
+----------------------------------------------------------+
|date_part(SECONDS, TIMESTAMP '2019-10-01 00:00:01.000001')|
+----------------------------------------------------------+
|                                                  1.000001|
+----------------------------------------------------------+

while mysql's is similar as this pr

# this is MYSQL
EXTRACT(SECOND FROM "2017-06-20 00:00:01.123456");
1

I originally purposed to output f64 instead of i32 since i'd like to follow postgresql's
I don't have strong opinion here, if stick with what MySQL is like, then i think i32 is enough

@alamb @tustvold do you have any suggestion?
note that this issue was from #3980 (comment)

comphead · 2022-11-28T22:53:37Z

Thanks @waitingkuo for testing the PR. I think since we now having seconds fraction support so we can concat the value and be in sync with postgres date_part if this is a preferred way

alamb · 2022-12-01T14:52:59Z

I think we should follow postgres (output floating point) if possible

# Conflicts: # datafusion/physical-expr/src/datetime_expressions.rs

…afusion into date_part_f64 # Conflicts: # datafusion/physical-expr/src/datetime_expressions.rs

comphead · 2022-12-06T06:10:51Z

@waitingkuo made in sync with postgres.
I have found extract nanosecond faces parser issue

DataFusion CLI v14.0.0
❯ select EXTRACT(nanosecond FROM to_timestamp('2020-09-08T12:00:12.12345678+00:00'));  🤔 Invalid statement: sql parser error: Expected date/time field, found: nanosecond

whereas nanosecond date_part works. I will create a ticket for this

waitingkuo

hi @comphead thank you, this looks good to me.

waitingkuo · 2022-12-06T08:35:38Z

datafusion/core/tests/sql/expr.rs

+    // test_expression!(
+    //     "EXTRACT(nanosecond FROM to_timestamp('2020-09-08T12:00:12.12345678+00:00'))",
+    //     "1212345678"
+    // );


i suggest that uncomment this test case and expect the error
e.g. like this https://github.com/apache/arrow-datafusion/blob/740a4fa2c6ba4b85875a433bb86e5b00435a5969/datafusion/core/tests/sql/timestamp.rs#L1071-L1074

waitingkuo · 2022-12-06T08:38:34Z

@waitingkuo made in sync with postgres. I have found extract nanosecond faces parser issue
DataFusion CLI v14.0.0
❯ select EXTRACT(nanosecond FROM to_timestamp('2020-09-08T12:00:12.12345678+00:00'));  🤔 Invalid statement: sql parser error: Expected date/time field, found: nanosecond
whereas nanosecond date_part works. I will create a ticket for this

@comphead sqlparser doesn't support nanosecond for now.
https://github.com/sqlparser-rs/sqlparser-rs/blob/813f4a2eff8f091b643058ac1a46a4fbffd96806/src/parser.rs#L1187-L1221

waitingkuo · 2022-12-06T10:14:18Z

@comphead i created a ticket and submitted a pr in sqlparser apache/datafusion-sqlparser-rs#748

it would be great if you could creat another ticket in datafusion so that we could fix it once the issue in sqlparser merged and new version released

comphead · 2022-12-06T16:09:50Z

@comphead i created a ticket and submitted a pr in sqlparser sqlparser-rs/sqlparser-rs#748

it would be great if you could creat another ticket in datafusion so that we could fix it once the issue in sqlparser merged and new version released

#4528
Please assign this to me.

alamb · 2022-12-06T21:17:10Z

Thanks @comphead and @waitingkuo !

ursabot · 2022-12-06T21:26:33Z

Benchmark runs are scheduled for baseline = 19cddf5 and contender = cedb05a. cedb05a 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

support seconds fraction in date_part

4234f22

github-actions bot added core Core DataFusion crate physical-expr Changes to the physical-expr crates labels Nov 27, 2022

comphead added 4 commits December 3, 2022 21:44

support seconds fraction in date_part

31eef92

Merge branch 'master' into date_part_f64

d99b917

# Conflicts: # datafusion/physical-expr/src/datetime_expressions.rs

to be insync with pgsql

3d53116

Merge branch 'date_part_f64' of https://github.com/comphead/arrow-dat…

c7df326

…afusion into date_part_f64 # Conflicts: # datafusion/physical-expr/src/datetime_expressions.rs

github-actions bot added the logical-expr Logical plan and expressions label Dec 6, 2022

comphead marked this pull request as ready for review December 6, 2022 06:01

fix merge

dc3ef88

waitingkuo approved these changes Dec 6, 2022

View reviewed changes

comphead mentioned this pull request Dec 6, 2022

Extract from timestamp doesn't support nanosecond #4528

Closed

comphead added 2 commits December 6, 2022 08:10

fix bench

684ed57

fix bench

46045ef

alamb merged commit cedb05a into apache:master Dec 6, 2022

comphead mentioned this pull request Dec 6, 2022

change extract return types to decimal to align with postgres #3996

Open

comphead deleted the date_part_f64 branch January 1, 2023 22:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`date_part` support fractions of second #4385

`date_part` support fractions of second #4385

comphead commented Nov 27, 2022

comphead commented Nov 27, 2022 •

edited

Loading

waitingkuo commented Nov 28, 2022 •

edited

Loading

comphead commented Nov 28, 2022

alamb commented Dec 1, 2022

comphead commented Dec 6, 2022

waitingkuo left a comment

waitingkuo Dec 6, 2022 •

edited

Loading

waitingkuo commented Dec 6, 2022

waitingkuo commented Dec 6, 2022

comphead commented Dec 6, 2022

alamb commented Dec 6, 2022

ursabot commented Dec 6, 2022

date_part support fractions of second #4385

date_part support fractions of second #4385

Conversation

comphead commented Nov 27, 2022

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

comphead commented Nov 27, 2022 • edited Loading

waitingkuo commented Nov 28, 2022 • edited Loading

comphead commented Nov 28, 2022

alamb commented Dec 1, 2022

comphead commented Dec 6, 2022

waitingkuo left a comment

Choose a reason for hiding this comment

waitingkuo Dec 6, 2022 • edited Loading

Choose a reason for hiding this comment

waitingkuo commented Dec 6, 2022

waitingkuo commented Dec 6, 2022

comphead commented Dec 6, 2022

alamb commented Dec 6, 2022

ursabot commented Dec 6, 2022

`date_part` support fractions of second #4385

`date_part` support fractions of second #4385

comphead commented Nov 27, 2022 •

edited

Loading

waitingkuo commented Nov 28, 2022 •

edited

Loading

waitingkuo Dec 6, 2022 •

edited

Loading