Skip to content
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

sql: bring EXTRACT into alignment with PostgreSQL v14 #9853

Closed
benesch opened this issue Jan 2, 2022 · 2 comments · Fixed by #10027
Closed

sql: bring EXTRACT into alignment with PostgreSQL v14 #9853

benesch opened this issue Jan 2, 2022 · 2 comments · Fixed by #10027

Comments

@benesch
Copy link
Member

benesch commented Jan 2, 2022

In PostgreSQL v14, EXTRACT now returns type numeric rather than float8. The date_part function continues to return float8. We should make the same change in Materialize, even though it'll be a bit annoying.

PostgreSQL commit: postgres/postgres@a2da77c

PostgreSQL discussion: https://www.postgresql.org/message-id/flat/42b73d2d-da12-ba9f-570a-420e0cce19d9@phystech.edu

While we're at it, we should also spruce up our docs on this front. We're missing mention of the date_part function entirely. The EXTRACT docs are awesome but don't mention that you can call EXTRACT with values of type time or interval, and also probably ought to go into more detail about the implicit conversions that happen and can produce surprising results (e.g., taking date to timestamp).

@jkosh44
Copy link
Contributor

jkosh44 commented Jan 3, 2022

Just a note for whoever implements this, you can find the Postgres documentation here: https://www.postgresql.org/docs/current/functions-datetime.html#FUNCTIONS-DATETIME-EXTRACT.

Despite what the documentation says, EXTRACT is explicitly implemented for the date type in Postgres(https://github.com/postgres/postgres/blob/dfe67c0e85a5a613f802f78641ccc1c48845076e/src/backend/utils/adt/date.c#L1068-L1242). This causes the units to be validated against the date type. Ex:

postgres=#  SELECT EXTRACT(second FROM date '2000-12-16 12:21:13');
ERROR:  date units "second" not supported

Currently Materialize does not implement EXTRACT for date explicitly. So the fix for this issue should probably change that.

materialize=> SELECT EXTRACT(second FROM date '2000-12-16 12:21:13');
 date_part 
-----------
         0
(1 row)

However DATE_PART is NOT explicitly implemented for the date type in Postgres. Instead it's cast to a timestamp like the documentation says. This causes the units to not be validated against the date type. Ex:

postgres=#  SELECT DATE_PART('second', date '2000-12-16 12:21:13');
 date_part 
-----------
         0
(1 row)

Currently this is the same behavior as Materialize.

materialize=> SELECT DATE_PART('second', date '2000-12-16 12:21:13');
 date_part 
-----------
         0
(1 row)

@jkosh44
Copy link
Contributor

jkosh44 commented Jan 11, 2022

If this isn't urgent can I give it a try?

I have a first pass here: jkosh44@8b12114

branch is here: https://github.com/jkosh44/materialize/tree/extract

Which seems to be working:

materialize=> select pg_typeof(date_part('epoch', '2001-09-09 01:46:40.000021'::timestamp));
    pg_typeof     
------------------
 double precision
(1 row)

materialize=> select pg_typeof(extract(epoch FROM '2001-09-09 01:46:40.000021'::timestamp));
 pg_typeof 
-----------
 numeric
(1 row)

I still need to:

  • Implement EXTRACT for Interval
  • Implement EXTRACT Epoch for Date and Time
  • Add tests
  • Clean up code and figure out ways to make code more concise
  • Add docs
  • Add release notes

jkosh44 added a commit to jkosh44/materialize that referenced this issue Jan 12, 2022
Before this commit EXTRACT was treated as an alias to date_part with slightly
different syntax. Both of these functions returned a float data type. PostgreSQL
v14 updated EXTRACT to return a numeric data type, which makes it compliant with
the SQL standard. This commit updates the EXTRACT function so that it return a
numeric data type so that it matches PostgreSQL. date_part still returns a float.

Additionally PostgreSQL v14 implemented EXTRACT explicitly for the DATE data
type. Previously DATEs were casted to TIMESTAMPs before extracting fields from
them. This commit also explicitly implements EXTRACT for DATE types, so they
aren't cast to a TIMESTAMP first. A consequence of this is that time related
fields (e.g. SECOND) are now rejected when trying to EXTRACT a DATE type.
However, The implementation for date_part still casts DATE types to TIMESTAMP
types. This means that date_part does not reject time related fields for DATE
types. This implementation matches PostgreSQL.

This commit also implements extracting EPOCHs from TIME types, which wasn't
previously implemented.

Fixes MaterializeInc#9853, MaterializeInc#9870
jkosh44 added a commit to jkosh44/materialize that referenced this issue Jan 12, 2022
Before this commit EXTRACT was treated as an alias to date_part with slightly
different syntax. Both of these functions returned a float data type. PostgreSQL
v14 updated EXTRACT to return a numeric data type, which makes it compliant with
the SQL standard. This commit updates the EXTRACT function so that it return a
numeric data type so that it matches PostgreSQL. date_part still returns a float.

Additionally PostgreSQL v14 implemented EXTRACT explicitly for the DATE data
type. Previously DATEs were casted to TIMESTAMPs before extracting fields from
them. This commit also explicitly implements EXTRACT for DATE types, so they
aren't cast to a TIMESTAMP first. A consequence of this is that time related
fields (e.g. SECOND) are now rejected when trying to EXTRACT a DATE type.
However, The implementation for date_part still casts DATE types to TIMESTAMP
types. This means that date_part does not reject time related fields for DATE
types. This implementation matches PostgreSQL.

This commit also implements extracting EPOCHs from TIME types, which wasn't
previously implemented.

Fixes MaterializeInc#9853, MaterializeInc#9870
jkosh44 added a commit to jkosh44/materialize that referenced this issue Jan 12, 2022
Before this commit EXTRACT was treated as an alias to date_part with slightly
different syntax. Both of these functions returned a float data type. PostgreSQL
v14 updated EXTRACT to return a numeric data type, which makes it compliant with
the SQL standard. This commit updates the EXTRACT function so that it return a
numeric data type so that it matches PostgreSQL. date_part still returns a float.

Additionally PostgreSQL v14 implemented EXTRACT explicitly for the DATE data
type. Previously DATEs were casted to TIMESTAMPs before extracting fields from
them. This commit also explicitly implements EXTRACT for DATE types, so they
aren't cast to a TIMESTAMP first. A consequence of this is that time related
fields (e.g. SECOND) are now rejected when trying to EXTRACT a DATE type.
However, The implementation for date_part still casts DATE types to TIMESTAMP
types. This means that date_part does not reject time related fields for DATE
types. This implementation matches PostgreSQL.

This commit also implements extracting EPOCHs from TIME types, which wasn't
previously implemented.

Fixes MaterializeInc#9853, MaterializeInc#9870
jkosh44 added a commit to jkosh44/materialize that referenced this issue Jan 13, 2022
Before this commit, EXTRACT was treated as an alias to date_part with slightly
different syntax. Both of these functions returned a float data type. PostgreSQL
v14 updated EXTRACT to return a numeric data type, which makes it compliant with
the SQL standard. This commit updates the EXTRACT function so that it return a
numeric data type so that it matches PostgreSQL. date_part still returns a float.

Additionally PostgreSQL v14 implemented EXTRACT explicitly for the DATE data
type. Previously DATEs were casted to TIMESTAMPs before extracting fields from
them. This commit also explicitly implements EXTRACT for DATE types, so they
aren't cast to a TIMESTAMP first. A consequence of this is that time related
fields (e.g. SECOND) are now rejected when trying to EXTRACT a DATE type.
However, The implementation for date_part still casts DATE types to TIMESTAMP
types. This means that date_part does not reject time related fields for DATE
types. This implementation matches PostgreSQL.

The postgres commit that implements these changes can be found here:
postgres/postgres@a2da77c

This commit also implements extracting EPOCHs from TIME types, which wasn't
previously implemented.

Fixes MaterializeInc#9853, MaterializeInc#9870
jkosh44 added a commit that referenced this issue Feb 2, 2022
Bring EXTRACT into alignment with PostgreSQL v14

Before this commit, EXTRACT was treated as an alias to date_part with slightly
different syntax. Both of these functions returned a float data type. PostgreSQL
v14 updated EXTRACT to return a numeric data type, which makes it compliant with
the SQL standard. This commit updates the EXTRACT function so that it return a
numeric data type so that it matches PostgreSQL. date_part still returns a float.

Additionally PostgreSQL v14 implemented EXTRACT explicitly for the DATE data
type. Previously DATEs were casted to TIMESTAMPs before extracting fields from
them. This commit also explicitly implements EXTRACT for DATE types, so they
aren't cast to a TIMESTAMP first. A consequence of this is that time related
fields (e.g. SECOND) are now rejected when trying to EXTRACT a DATE type.
However, The implementation for date_part still casts DATE types to TIMESTAMP
types. This means that date_part does not reject time related fields for DATE
types. This implementation matches PostgreSQL.

The postgres commit that implements these changes can be found here:
postgres/postgres@a2da77c

This commit also implements extracting EPOCHs from TIME types, which wasn't
previously implemented.

Fixes #9853, #9870, #10027
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants