[SPARK-28687][SQL] Support `epoch`, `isoyear`, `milliseconds` and `microseconds` at `extract()` by MaxGekk · Pull Request #25408 · apache/spark

MaxGekk · 2019-08-11T14:10:08Z

What changes were proposed in this pull request?

In the PR, I propose new expressions Epoch, IsoYear, Milliseconds and Microseconds, and support additional parameters of extract() for feature parity with PostgreSQL (https://www.postgresql.org/docs/11/functions-datetime.html#FUNCTIONS-DATETIME-EXTRACT):

epoch - the number of seconds since 1970-01-01 00:00:00 local time in microsecond precision.
isoyear - the ISO 8601 week-numbering year that the date falls in. Each ISO 8601 week-numbering year begins with the Monday of the week containing the 4th of January.
milliseconds - the seconds field including fractional parts multiplied by 1,000.
microseconds - the seconds field including fractional parts multiplied by 1,000,000.

Here are examples:

spark-sql> SELECT EXTRACT(EPOCH FROM TIMESTAMP '2019-08-11 19:07:30.123456');
1565550450.123456
spark-sql> SELECT EXTRACT(ISOYEAR FROM DATE '2006-01-01');
2005
spark-sql> SELECT EXTRACT(MILLISECONDS FROM TIMESTAMP '2019-08-11 19:07:30.123456');
30123.456
spark-sql> SELECT EXTRACT(MICROSECONDS FROM TIMESTAMP '2019-08-11 19:07:30.123456');
30123456

How was this patch tested?

Added new tests to DateExpressionsSuite, and uncommented existing tests in extract.sql and pgSQL/date.sql.

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala

sql/core/src/test/resources/sql-tests/inputs/pgSQL/date.sql

SparkQA · 2019-08-11T17:45:18Z

Test build #108930 has finished for PR 25408 at commit d16a6ed.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-08-11T21:40:42Z

Test build #108936 has finished for PR 25408 at commit f152004.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala

SparkQA · 2019-08-12T07:05:02Z

Test build #108954 has finished for PR 25408 at commit f025e6e.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun

This PR's milliseconds implementation is inconsistent from PostgreSQL. It must be due to a simple misunderstanding on the contracts.

PostgreSQL

postgres=# select extract(milliseconds from timestamp '2011-05-06 07:08:09.1234567');
 date_part
-----------
  9123.457
(1 row)

This PR

spark-sql> select extract(milliseconds from timestamp '2011-05-06 07:08:09.1234567');
9123

Could you fix that and double-check all the other function implementations once again by comparing the full sub-second parts? Also, the document should be updated together to avoid this kind of confusions at user-sides.

In addition, PostgreSQL accepts more patterns like MSEC. Since this is for PostgreSQL Feature Parity, we had better support all patterns.

postgres=# SELECT EXTRACT(MSEC FROM TIMESTAMP   '2011-05-06 07:08:09.1234567');
 date_part
-----------
  9123.457
(1 row)

SparkQA · 2019-08-12T10:40:21Z

Test build #108956 has finished for PR 25408 at commit c0dea7b.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

MaxGekk · 2019-08-12T18:36:17Z

@dongjoon-hyun Just in case, you checked not only behavior of extract but parsing string to timestamp as well. I mean rounding .1234567 to .123456. After the commit 3c848e7 :

spark-sql> select extract(milliseconds from timestamp '2011-05-06 07:08:09.1234567');
9123.456

dongjoon-hyun · 2019-08-12T19:43:51Z

Thanks, @MaxGekk . BTW, please don't forget the other part like MSECsupport in my comment.

SparkQA · 2019-08-12T22:13:18Z

Test build #108990 has finished for PR 25408 at commit 3c848e7.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-08-13T07:05:02Z

Test build #109017 has finished for PR 25408 at commit a317a64.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

MaxGekk · 2019-08-13T07:10:24Z

jenkins, retest this, please

SparkQA · 2019-08-13T08:45:14Z

Test build #109024 has finished for PR 25408 at commit a317a64.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

MaxGekk · 2019-08-13T09:07:31Z

jenkins, retest this, please

SparkQA · 2019-08-13T12:34:54Z

Test build #109033 has finished for PR 25408 at commit a317a64.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2019-08-13T15:45:12Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

Ur, @MaxGekk . Did you check PostgreSQL to find out all supported variants?
Please check the variants really instead of relying on my comments. 😅

Let me be clear. I didn't check PostgreSQL full variants, but, at least, MS and MSECS are missing here.

cc @maropu . Could you help @MaxGekk to complete this feature?

Did you check PostgreSQL to find out all supported variants?

I checked PostgreSQL documentation and JIRA ticket that mention milliseconds only. As far as I see timestamp.sql uses msec / msec as well.

at least, MS and MSECS are missing here.

Do we need to support them if they are not used in tests?

Please check the variants really instead of relying on my comments. 😅

I relied on existing test

spark/sql/core/src/test/resources/sql-tests/inputs/pgSQL/timestamp.sql

Lines 198 to 199 in caa23e3

-- date_part( 'quarter', d1) AS quarter, date_part( 'msec', d1) AS msec,

-- date_part( 'usec', d1) AS usec

not on your comment, sorry :-E

I've checked the related code in pgsql and it seems to have many variants:

millisecon

ms

msec

mseconds

msecs

msecond

The implementation of extract is timestamp_part in pgsql: https://github.com/postgres/postgres/blob/master/src/backend/utils/adt/timestamp.c#L4507
This function handles DTK_MILLISEC for milleseconds: https://github.com/postgres/postgres/blob/fded4773eb60541c6e7dbcf09c9bcb1cd36a063b/src/backend/utils/adt/timestamp.c#L4552
DTK_MILLISEC is expanded into many variants above:
https://github.com/postgres/postgres/blob/fded4773eb60541c6e7dbcf09c9bcb1cd36a063b/src/interfaces/ecpg/pgtypeslib/dt_common.c#L446
https://github.com/postgres/postgres/blob/fded4773eb60541c6e7dbcf09c9bcb1cd36a063b/src/include/utils/datetime.h#L49

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

MaxGekk · 2019-08-13T16:49:31Z

@dongjoon-hyun It seems there are many synonyms for the same units in PostgreSQL: https://github.com/postgres/postgres/blob/64579be64a3811d08ea7ccf92b1a443d76b96412/src/backend/utils/adt/datetime.c#L171-L234 . Are you sure we must support all of them?

SparkQA · 2019-08-13T20:59:48Z

Test build #109051 has finished for PR 25408 at commit 7fcb6a4.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2019-08-13T23:34:35Z

Could you rebase this to the master please, @MaxGekk ?

SparkQA · 2019-08-14T10:29:42Z

Test build #109091 has finished for PR 25408 at commit 153d2ff.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun

+1, LGTM. Thank you, @MaxGekk , @maropu , @HyukjinKwon .
Merged to master!

MaxGekk added 10 commits August 9, 2019 15:58

Support microseconds arg at extract

3991b67

Support milliseconds arg at extract

330a5f2

Support epoch

9c7de57

Switch to decimal instead of double

0186274

Support isoyear at extract

7bbfe1f

Fix test for getEpoch

90ebddb

Remove unused import

296250e

Tests for microseconds and milliseconds

b97aadb

Merge remote-tracking branch 'remotes/origin/master' into extract-ext3

97a2c00

Re-gen results for date.sql

d16a6ed