BUG #18348: Inconsistency with EXTRACT([field] from INTERVAL);

Adding another inconsistency I found in the docs to this thread (
https://www.postgresql.org/docs/current/functions-datetime.html#FUNCTIONS-DATETIME-EXTRACT
):

The docs say: "source must be a value expression of type *timestamp*, *time*,
or *interval*. (Expressions of type *date* are *cast to timestamp* and can
therefore be used as well.)"

Which implies that the following two results must be the same:

tpch=# select extract(microseconds from date '1924.01.01');
ERROR: date units "microseconds" not supported

tpch=# select extract(microseconds from (date '1924.01.01')::timestamp);
extract
---------
0
(1 row)

However, the behaviour is different, which suggests that the date is indeed
treated as its own type in EXTRACT, and not cast to timestamp.

On Fri, Feb 16, 2024 at 2:07 PM PG Bug reporting form <
noreply@postgresql.org> wrote:

On Sat, 17 Feb 2024 at 01:27, Michael Bondarenko
<work.michael.2956@gmail.com> wrote:

Adding another inconsistency I found in the docs to this thread (https://www.postgresql.org/docs/current/functions-datetime.html#FUNCTIONS-DATETIME-EXTRACT):

The docs say: "source must be a value expression of type timestamp, time, or interval. (Expressions of type date are cast to timestamp and can therefore be used as well.)"

Which implies that the following two results must be the same:

tpch=# select extract(microseconds from date '1924.01.01');
ERROR: date units "microseconds" not supported

tpch=# select extract(microseconds from (date '1924.01.01')::timestamp);
extract
---------
0

It looks like a2da77cdb should have updated the documentation for this.

David

On Sat, 17 Feb 2024 at 01:27, PG Bug reporting form
<noreply@postgresql.org> wrote:

tpch=# select extract(week from interval '3 weeks');
ERROR: interval units "week" not supported

In the documentation it's mentioned that 'week' is an ISO 8601 week, so it
makes sense why it's not applicable to INTERVAL, which is the same for
isoyear. However, the field is named week and not isoweek, so I expect it to
work like the `select extract(year from interval '3 years');` does.
Moreover, the documentation does not mention that the field cannot be
extracted from INTERVAL, like it does for isoyear:
https://www.postgresql.org/docs/current/functions-datetime.html#FUNCTIONS-DATETIME-EXTRACT

Maybe that table should specify which type(s) each of the items listed
is applicable to. Seems better than mentioning which types they're not
applicable to.

David

in `9.9.1. EXTRACT, date_part`
EXTRACT(field FROM source)

I saw more inconsistencies with the doc when `source` is an interval.

the `minute` field
select extract(minute from interval '2011 year 16 month 35 day 48 hour
1005 min 71 sec 11 ms');
select extract(minute from interval '2011 year 16 month 35 day 48 hour
1005 min 2 sec 11 ms');
select extract(minute from interval '2011 year 16 month 35 day 48 hour
1005 min 2 sec 11 ms');

the `hour` field:
select extract(hour from interval '2011 year 16 month 35 day 48 hour
1005 min 71 sec 11 ms');
select extract(hour from interval '2011 year 16 month 35 day 48 hour
1005 min 2 sec 11 ms');
select extract(hour from interval '2011 year 16 month 35 day 48 hour
1005 min 71 sec 11111111111 ms');

the `quarter` field:
select extract(quarter from interval '2011 year 12 month 48 hour 1005
min 2 sec 11 ms');
SELECT EXTRACT(QUARTER FROM TIMESTAMP '2001-12-16 20:38:40');

When testing I stumbled upon that too, but I thought no calculation was
happening in the interval field. However, it's different with the days and
months etc. It seems no calculation for day and month and more:

tpch=# select extract(day from interval '86400000 seconds');
extract
---------
0
(1 row)

tpch=# select extract(month from interval '86400000 seconds');
extract
---------
0
(1 row)

tpch=# select extract(year from interval '86400000 seconds');
extract
---------
0
(1 row)

But calculation is present for hour, and minutes and seconds (90061 sec is
1 day 1 hour 1 minute 1 second):

tpch=# select extract(minute from interval '90061 seconds');
extract
---------
1
(1 row)

tpch=# select extract(hour from interval '90061 seconds');
extract
---------
25
(1 row)

tpch=# select extract(second from interval '90061 seconds');
extract
----------
1.000000
(1 row)

The docs mention *The hour field (0–23)* for the hours, which is not true
because it's not the field at all, but the calculated amount, and the value
is not 0-23.

On Sat, Feb 17, 2024 at 3:48 AM jian he <jian.universality@gmail.com> wrote:

On Sat, 17 Feb 2024 at 09:01, Michael Bondarenko
<work.michael.2956@gmail.com> wrote:

When testing I stumbled upon that too, but I thought no calculation was happening in the interval field. However, it's different with the days and months etc. It seems no calculation for day and month and more:

...

But calculation is present for hour, and minutes and seconds (90061 sec is 1 day 1 hour 1 minute 1 second):

No, intervals have seconds, days and months. This is because not all
days have 24 hours, due to DST they can have 23 or 25, or even more
extreme values if some country decides to change its time zone
definition. And not all months have 30 days, so 90061 is 0 months, 0
days, 25 hours, 1 minute, 1 second ( IIRC leap second are not handled
).

It is done that way so when you add one day across a dst jump you get
the same hour on the next day, and when you add one month you get the
same day in the next month independent of how many days the month has.
This is great for things like "schedule a meeting one month and one
week from now", but it bites you sometimes, like when you need a
duration to bill for a long event like a phone call, where I always
end up extracting epoch and substracting them.

Francisco Olarte.

David Rowley <dgrowleyml@gmail.com> writes:

On Sat, 17 Feb 2024 at 01:27, PG Bug reporting form
<noreply@postgresql.org> wrote:

Moreover, the documentation does not mention that the field cannot be
extracted from INTERVAL, like it does for isoyear:
https://www.postgresql.org/docs/current/functions-datetime.html#FUNCTIONS-DATETIME-EXTRACT

Maybe that table should specify which type(s) each of the items listed
is applicable to. Seems better than mentioning which types they're not
applicable to.

The thing's not laid out as a table though, and converting it seems
like more trouble than this is worth. The rejected cases hardly seem
surprising. I propose just mentioning that not all fields apply for
all data types, as in 0001 attached.

(Parenthetically, one case that perhaps is surprising is
ERROR: unit "week" not supported for type interval
Why not just return the day field divided by 7?)

Unrelated but adjacent, the discussion of the century field seems
more than a bit flippant when I read it now. In other places we
are typically content to use examples to make similar points.
I propose doing so here too, as in 0002 attached.

Lastly, the entire page is quite schizophrenic about whether to leave
a blank line between adjacent examples. I could go either way on
whether to have that whitespace or not, but I do think it would be
better to make it uniform. Any votes on what to do there?

regards, tom lane

Francisco Olarte <folarte@peoplecall.com> writes:

On Sat, 17 Feb 2024 at 09:01, Michael Bondarenko
<work.michael.2956@gmail.com> wrote:

When testing I stumbled upon that too, but I thought no calculation was happening in the interval field. However, it's different with the days and months etc. It seems no calculation for day and month and more:
...
But calculation is present for hour, and minutes and seconds (90061 sec is 1 day 1 hour 1 minute 1 second):

No, intervals have seconds, days and months.

Yeah. I think much of the confusion here comes from starting with
non-normalized interval input. Sure you can write "2011 year 12 month
48 hour 1005 min 2 sec 11 ms", but that's not how it's stored:

regression=# select interval '2011 year 12 month 48 hour 1005 min 2 sec 11 ms';
interval
-------------------------
2012 years 64:45:02.011
(1 row)

(Actually, what's stored is 2012*12 months, 0 days, and some number
of microseconds that I don't feel like working out. Conversion of
the microseconds to HH:MM:SS.SSS happens on output.)

Once you look at the normalized value, the results of extract()
are far less surprising.

Probably the right place to enlarge on this point is not in the
extract() section at all, but in 8.5.4. Interval Input. That does
mention the months/days/microseconds representation, but it doesn't
follow through by illustrating how other input is combined. Perhaps
we'd want to adopt something like the attached (this is separate from
the other patches I posted in the thread).

regards, tom lane

On Sun, Feb 18, 2024 at 4:30 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Once you look at the normalized value, the results of extract()
are far less surprising.

Probably the right place to enlarge on this point is not in the
extract() section at all, but in 8.5.4. Interval Input. That does
mention the months/days/microseconds representation, but it doesn't
follow through by illustrating how other input is combined. Perhaps
we'd want to adopt something like the attached (this is separate from
the other patches I posted in the thread).

--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -10040,13 +10040,19 @@ EXTRACT(<replaceable>field</replaceable>
FROM <replaceable>source</replaceable>)
     The <function>extract</function> function retrieves subfields
     such as year or hour from date/time values.
     <replaceable>source</replaceable> must be a value expression of
-    type <type>timestamp</type>, <type>time</type>, or <type>interval</type>.
-    (Expressions of type <type>date</type> are
-    cast to <type>timestamp</type> and can therefore be used as
-    well.)  <replaceable>field</replaceable> is an identifier or
+    type <type>timestamp</type>, <type>date</type>, <type>time</type>,
+    or <type>interval</type>.  (Timestamps and times can be with or
+    without time zone.)
+    <replaceable>field</replaceable> is an identifier or
     string that selects what field to extract from the source value.
+    Not all fields are valid for every input data type; for example, fields
+    smaller than a day cannot be extracted from a <type>date</type>, while
+    fields of a day or more cannot be extracted from a <type>time</type>.
     The <function>extract</function> function returns values of type
     <type>numeric</type>.
+   </para>

you already mentioned "Not all fields are valid for every input data type".
interval data type don't even have a unit "quarter",
so the following should generate an error?
select extract(quarter from interval '2011 year 12 month 48 hour
1005min 2 sec 11 ms');

9.9.1. EXTRACT, date_part
hour field description as
`
The hour field (0–23)
`
Do we need to update for the EXTRACT(INTERVAL) case?

jian he <jian.universality@gmail.com> writes:

you already mentioned "Not all fields are valid for every input data type".
interval data type don't even have a unit "quarter",
so the following should generate an error?
select extract(quarter from interval '2011 year 12 month 48 hour
1005min 2 sec 11 ms');

I'm not especially persuaded by that reasoning. Intervals don't have
century or millisecond fields either, but we allow extracting those.

If your argument is that we shouldn't allow it because we don't take
the input INTERVAL '1 quarter', I'd be much more inclined to add that
as valid input than to take away existing extract functionality.
But I'm dubious about the proposition that extract's list of valid
fields should exactly match the set of allowed input units. The
semantics aren't really the same (as per the '80 minutes' example)
so such a restriction doesn't seem to have much basis in reality.

regards, tom lane

jian he <jian.universality@gmail.com> writes:

9.9.1. EXTRACT, date_part
hour field description as
`
The hour field (0–23)
`
Do we need to update for the EXTRACT(INTERVAL) case?

Yeah, probably. I did a bit more wordsmithing too.
Here's a rolled-up patch.

regards, tom lane

On Sun, Feb 18, 2024 at 2:14 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

(Parenthetically, one case that perhaps is surprising is
ERROR: unit "week" not supported for type interval
Why not just return the day field divided by 7?)

seems pretty simple?
diff --git a/src/backend/utils/adt/timestamp.c
b/src/backend/utils/adt/timestamp.c
index ed03c50a..5e69e258 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -5992,6 +5992,10 @@ interval_part_common(PG_FUNCTION_ARGS, bool retnumeric)
                                intresult = tm->tm_mday;
                                break;

+                       case DTK_WEEK:
+                               intresult = (tm->tm_mday  - 1) / 7 + 1;
+                               break;
but I am not sure not sure how to write the doc.

On Sun, Feb 18, 2024 at 10:19 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

jian he <jian.universality@gmail.com> writes:

you already mentioned "Not all fields are valid for every input data type".
interval data type don't even have a unit "quarter",
so the following should generate an error?
select extract(quarter from interval '2011 year 12 month 48 hour
1005min 2 sec 11 ms');

I'm not especially persuaded by that reasoning. Intervals don't have
century or millisecond fields either, but we allow extracting those.

If your argument is that we shouldn't allow it because we don't take
the input INTERVAL '1 quarter', I'd be much more inclined to add that
as valid input than to take away existing extract functionality.
But I'm dubious about the proposition that extract's list of valid
fields should exactly match the set of allowed input units. The
semantics aren't really the same (as per the '80 minutes' example)
so such a restriction doesn't seem to have much basis in reality.

in interval_part_common:
case DTK_QUARTER:
intresult = (tm->tm_mon / 3) + 1;
break;

in timestamp_part_common:
case DTK_QUARTER:
intresult = (tm->tm_mon - 1) / 3 + 1;
break;

So in section 9.9.1. EXTRACT, date_part
we may need to document extract(quarter from interval) case.
intervals can be negative, which will make the issue more complicated.
except the "quarter" field , EXTRACT other fields from intervals, the
output seems sane.

for example:
drop table s;
create table s(a interval);
insert into s select ( g * 1000 || 'year ' || g || 'month ' || g || '
day ' || g || 'hour ' || g || 'min ' || g || 'sec' )::interval
from generate_series(-20, 20) g;

select
extract(century from a) as century,
extract(millennium from a) as millennium,
extract(decade from a) as decade,
extract(year from a) as year,
extract(quarter from a) as quarter,
extract(month from a) as mon,
extract(day from a) as day,
extract(hour from a) as hour,
extract(min from a) as min,
extract(second from a) as sec,
extract(microseconds from a) as microseconds
-- a
from s order by 2 asc;

jian he <jian.universality@gmail.com> writes:

On Sun, Feb 18, 2024 at 2:14 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

(Parenthetically, one case that perhaps is surprising is
ERROR: unit "week" not supported for type interval
Why not just return the day field divided by 7?)

seems pretty simple?

Hm, maybe, but does this behave desirably for zero or negative days?

So in section 9.9.1. EXTRACT, date_part
we may need to document extract(quarter from interval) case.
intervals can be negative, which will make the issue more complicated.
except the "quarter" field , EXTRACT other fields from intervals, the
output seems sane.

Yeah, I see what you mean: the output for negative month counts is
very bizarre, whereas other fields seem to all produce the negative
of what they'd produce for the absolute value of the interval.
We could either try to fix that or decide that rejecting "quarter"
for intervals is the saner answer.

I went ahead and pushed the docs changes after adding more explicit
descriptions of interval's behavior for the field types where it
seemed important. If we make any changes to the behavior for
week or quarter fields, ISTM that should be a HEAD-only change.

regards, tom lane

I wrote:

jian he <jian.universality@gmail.com> writes:

On Sun, Feb 18, 2024 at 2:14 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

(Parenthetically, one case that perhaps is surprising is
ERROR: unit "week" not supported for type interval
Why not just return the day field divided by 7?)

seems pretty simple?

Hm, maybe, but does this behave desirably for zero or negative days?

So in section 9.9.1. EXTRACT, date_part
we may need to document extract(quarter from interval) case.
intervals can be negative, which will make the issue more complicated.
except the "quarter" field , EXTRACT other fields from intervals, the
output seems sane.

Yeah, I see what you mean: the output for negative month counts is
very bizarre, whereas other fields seem to all produce the negative
of what they'd produce for the absolute value of the interval.
We could either try to fix that or decide that rejecting "quarter"
for intervals is the saner answer.

After fooling with these cases for a little I'm inclined to think
we should do it as attached (no test or docs changes yet).

regards, tom lane

On Wed, Feb 21, 2024 at 4:56 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

I wrote:

jian he <jian.universality@gmail.com> writes:

On Sun, Feb 18, 2024 at 2:14 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

(Parenthetically, one case that perhaps is surprising is
ERROR: unit "week" not supported for type interval
Why not just return the day field divided by 7?)

seems pretty simple?

Hm, maybe, but does this behave desirably for zero or negative days?

So in section 9.9.1. EXTRACT, date_part
we may need to document extract(quarter from interval) case.
intervals can be negative, which will make the issue more complicated.
except the "quarter" field , EXTRACT other fields from intervals, the
output seems sane.

Yeah, I see what you mean: the output for negative month counts is
very bizarre, whereas other fields seem to all produce the negative
of what they'd produce for the absolute value of the interval.
We could either try to fix that or decide that rejecting "quarter"
for intervals is the saner answer.

After fooling with these cases for a little I'm inclined to think
we should do it as attached (no test or docs changes yet).

regards, tom lane

for `week`, we can do following for the doc:

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index e5fa82c1..a21eb9f8 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -10422,7 +10422,7 @@ SELECT EXTRACT(SECOND FROM TIME '17:12:28.5');
         The number of the <acronym>ISO</acronym> 8601 week-numbering week of
         the year.  By definition, ISO weeks start on Mondays and the first
         week of a year contains January 4 of that year.  In other words, the
-        first Thursday of a year is in week 1 of that year.
+        first Thursday of a year is in week 1 of that year. For
<type>interval</type> values, divide the number of days by 7.

Actually, it's not totally correct, since "the number of days is a
numeric value. need to cast "the number of days" to int.

for positive interval value, we can
+ For positive <type>interval</type> values, divide the number of days
by 3 then plus 1.
I don't know how to write the documentation for the `quarter` when
it's negative.

jian he <jian.universality@gmail.com> writes:

On Wed, Feb 21, 2024 at 4:56 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Yeah, I see what you mean: the output for negative month counts is
very bizarre, whereas other fields seem to all produce the negative
of what they'd produce for the absolute value of the interval.
We could either try to fix that or decide that rejecting "quarter"
for intervals is the saner answer.

After fooling with these cases for a little I'm inclined to think
we should do it as attached (no test or docs changes yet).

... I don't know how to write the documentation for the `quarter` when
it's negative.

After poking at it some more, I realized that my draft patch was still
wrong about that. We really have to look at interval->month if we
want to behave plausibly for negative months.

Here's a more fleshed-out patch. I don't think we really need to
document the behavior for negative intervals; at least, we haven't
done that so far for any other fields. I did add testing of such
cases though.

regards, tom lane

Devils advocating here, feel free to ignore.

Is there a real need for a negative month? Sounds like high level this could be disastrous if I screw up the syntax. (Ah, memories of DD)

I have done this in data warehousing with dimensions tables.

Just process on the INT and translate into the name.

I was thinking on how a negative month could impact this side (data warehousing) side of querying.

I could be chicken little on this, but wanted it in the conversation.

workaround for negative months:

CREATE TABLE dim_biz_hours( year INT(4)
, doy INT(3)
, dow INT(7)
, month INT(2)
, day INT(2)
, hour INT(2)
, minute INT(2)
, second INT(2)
, utc_offset INT(2)
, utc_offset_dst INT(2)
);

INSERT INTO biz_hours (year)
SELECT * FROM generate_series(2000, 2099);

INSERT INTO biz_hours (doy)
SELECT * FROM generate_series(1, 366);

INSERT INTO biz_hours (dow)
SELECT * FROM generate_series(1, 7);

INSERT INTO biz_hours (month)
SELECT * FROM generate_series(1, 12);

INSERT INTO biz_hours (day)
SELECT * FROM generate_series(1, 31) ;

INSERT INTO biz_hours (hour)
SELECT * FROM generate_series(1, 24);

INSERT INTO biz_hours (minute)
SELECT * FROM generate_series(1, 60);

INSERT INTO biz_hours (second
SELECT * FROM generate_series(1, 60);

INSERT INTO biz_hours (utc_offset)
SELECT * FROM generate_series(1, 24);

INSERT INTO biz_hours (utc_offset_dst)
SELECT * FROM generate_series(1, 24);

-----Original Message-----
From: Tom Lane <tgl@sss.pgh.pa.us>
Sent: Tuesday, May 7, 2024 2:27 PM
To: jian he <jian.universality@gmail.com>
Cc: Francisco Olarte <folarte@peoplecall.com>; Michael Bondarenko <work.michael.2956@gmail.com>; pgsql-bugs@lists.postgresql.org; dgrowleyml@gmail.com; Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Subject: [EXTERNAL] Re: BUG #18348: Inconsistency with EXTRACT([field] from INTERVAL);

jian he <jian.universality@gmail.com> writes:

On Wed, Feb 21, 2024 at 4:56 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Yeah, I see what you mean: the output for negative month counts is
very bizarre, whereas other fields seem to all produce the negative
of what they'd produce for the absolute value of the interval.
We could either try to fix that or decide that rejecting "quarter"
for intervals is the saner answer.

After fooling with these cases for a little I'm inclined to think we
should do it as attached (no test or docs changes yet).

... I don't know how to write the documentation for the `quarter` when
it's negative.

After poking at it some more, I realized that my draft patch was still wrong about that. We really have to look at interval->month if we want to behave plausibly for negative months.

Here's a more fleshed-out patch. I don't think we really need to document the behavior for negative intervals; at least, we haven't done that so far for any other fields. I did add testing of such cases though.

regards, tom lane

On Wed, May 8, 2024 at 5:27 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Here's a more fleshed-out patch. I don't think we really need to
document the behavior for negative intervals; at least, we haven't
done that so far for any other fields. I did add testing of such
cases though.

the doc looks good to me.
extract quarter from the interval makes sense to me.

but in real life, for week, we generally begin with 1?
like "the first week", "second week"

so should
select extract(week from interval '1 day');
return 1
?

"Wetmore, Matthew (CTR)" <Matthew.Wetmore@evernorth.com> writes:

Devils advocating here, feel free to ignore.
Is there a real need for a negative month? Sounds like high level this could be disastrous if I screw up the syntax. (Ah, memories of DD)

What are you objecting to the "need for"? That intervals can store
negative months at all? I think that ship sailed a couple decades
ago. It's hard to use interval as the output of, say,
timestamp minus timestamp if it refuses to allow negative values.

The next fallback position perhaps could be that extract(quarter ...)
could throw error for negative input, but that seems like mostly a
foot-gun. We've striven elsewhere to not have it throw error, even
if there's not any very sane choice to make. For instance, these
are pre-existing behaviors:

regression=# select extract(quarter from interval 'infinity');
extract
---------

(1 row)

regression=# select extract(quarter from interval '-infinity');
extract
---------

(1 row)

Maybe there's a case for returning null for "quarter" for any negative
months value, but that seems inconsistent with other behaviors of
extract(). The pattern I see for finite values is that negating
the input interval negates each output of extract().

regards, tom lane