pg.ddx.io pgsql-bugs@postgresql.org mailing list archive help / color / mirror / Atom feed From: Tom Lane <tgl@sss.pgh.pa.us>
To: jian he <jian.universality@gmail.com>
Cc: Francisco Olarte <folarte@peoplecall.com>
Cc: Michael Bondarenko <work.michael.2956@gmail.com>
Cc: pgsql-bugs@lists.postgresql.org, dgrowleyml@gmail.com, Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Subject: Re: BUG #18348: Inconsistency with EXTRACT([field] from INTERVAL);
Date: Mon, 08 Jul 2024 13:03:28 -0400
Message-ID: <1079670.1720458208@sss.pgh.pa.us> (raw )
In-Reply-To: <476283.1715177448@sss.pgh.pa.us >
References: <18348-b097a3587dfde8a4@postgresql.org >
<CAApHDvr_-05ce6FaLM0P=hJBdV=Ht3=sdk_Eq__7YOf+fs5x9g@mail.gmail.com >
<CACJufxHYQSKNnt8rS7GxvK04wf75nw_ugMaibFVVfrcdVMmF1Q@mail.gmail.com >
<CAF_O+z1FU5GN=LnW50QjANGVB8+6VxyK1JHa8fE-Xs205MFuqw@mail.gmail.com >
<CA+bJJbw6q=6no1Hf67FraYGMo12asWCa7yw=J0zg_ELbivX0gw@mail.gmail.com >
<3709307.1708201825@sss.pgh.pa.us >
<CACJufxGB4MD6vRz3jVGK6kza-6fuDgfW2jC_1S2+MwcQYGkgSA@mail.gmail.com >
<3749306.1708222748@sss.pgh.pa.us >
<CACJufxHL0g73FPaRS+OGSYWJ0YbP6nEdVK79=wjL-TKU0_heDw@mail.gmail.com >
<222870.1708458150@sss.pgh.pa.us >
<270582.1708462564@sss.pgh.pa.us >
<CACJufxFOjGpAQJ5EJ0=a1geERgdHW77SEdRyK8A_qwB3CVdwhQ@mail.gmail.com >
<166456.1715117228@sss.pgh.pa.us >
<CACJufxGFuZsj+31a204fFpxv_RCHA-u1h6WEBZ6qpjBVoio5dQ@mail.gmail.com >
<476283.1715177448@sss.pgh.pa.us >
I took another look at this issue and got annoyed by the fact that the
proposed coding for "quarter" still doesn't satisfy the rule that
the output for a negative interval should be the negative of the
output for the sign-reversed interval. Specifically, if the month
field is zero, the v2 patch always emits 1:
regression=# select extract(quarter from interval '1 day');
extract
---------
1
(1 row)
regression=# select extract(quarter from interval '-1 day');
extract
---------
1
(1 row)
We could fix that by examining the sign of the lower-order fields
when month is zero, as in the v3 patch attached. However, I'm not
at all sure this is really better than v2. Notably, it makes the
documentation's statement that the result is "the month field
divided by 3 plus 1" even more incomplete. I still don't really
want to go into details about the behavior for negative intervals.
OTOH if we did do that, I'd rather write a blanket statement
about the result being the negative of the result for a positive
interval.
Thoughts?
regards, tom lane
Attachments:
[text/x-diff] v3-interval-week-and-quarter-fixes.patch (13.7K, ../1079670.1720458208@sss.pgh.pa.us/2-v3-interval-week-and-quarter-fixes.patch)
download | inline diff:
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 93ee3d4b60..bd430e712d 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -10392,12 +10392,16 @@ SELECT EXTRACT(MONTH FROM INTERVAL '2 years 13 months');
<term><literal>quarter</literal></term>
<listitem>
<para>
- The quarter of the year (1–4) that the date is in
+ The quarter of the year (1–4) that the date is in;
+ for <type>interval</type> values, the month field divided by 3
+ plus 1
</para>
<screen>
SELECT EXTRACT(QUARTER FROM TIMESTAMP '2001-02-16 20:38:40');
<lineannotation>Result: </lineannotation><computeroutput>1</computeroutput>
+SELECT EXTRACT(QUARTER FROM INTERVAL '1 year 6 months');
+<lineannotation>Result: </lineannotation><computeroutput>3</computeroutput>
</screen>
</listitem>
</varlistentry>
@@ -10468,9 +10472,16 @@ SELECT EXTRACT(SECOND FROM TIME '17:12:28.5');
<literal>week</literal> to get consistent results.
</para>
+ <para>
+ For <type>interval</type> values, the week field is simply the number
+ of integral days divided by 7.
+ </para>
+
<screen>
SELECT EXTRACT(WEEK FROM TIMESTAMP '2001-02-16 20:38:40');
<lineannotation>Result: </lineannotation><computeroutput>7</computeroutput>
+SELECT EXTRACT(WEEK FROM INTERVAL '13 days 24 hours');
+<lineannotation>Result: </lineannotation><computeroutput>1</computeroutput>
</screen>
</listitem>
</varlistentry>
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index cdc7e43b93..13ddba4674 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -5918,6 +5918,7 @@ NonFiniteIntervalPart(int type, int unit, char *lowunits, bool isNegative)
case DTK_MILLISEC:
case DTK_SECOND:
case DTK_MINUTE:
+ case DTK_WEEK:
case DTK_MONTH:
case DTK_QUARTER:
return 0.0;
@@ -6037,12 +6038,33 @@ interval_part_common(PG_FUNCTION_ARGS, bool retnumeric)
intresult = tm->tm_mday;
break;
+ case DTK_WEEK:
+ intresult = tm->tm_mday / 7;
+ break;
+
case DTK_MONTH:
intresult = tm->tm_mon;
break;
case DTK_QUARTER:
- intresult = (tm->tm_mon / 3) + 1;
+
+ /*
+ * We want to maintain the rule that a field extracted from a
+ * negative interval is the negative of the field's value for
+ * the sign-reversed interval. The broken-down tm_year and
+ * tm_mon aren't very helpful for that, so work from
+ * interval->month. If the month field is zero, use the
+ * lower-order fields to decide the sign.
+ */
+ if (interval->month > 0)
+ intresult = (tm->tm_mon / 3) + 1;
+ else if (interval->month < 0)
+ intresult = -(((-interval->month % MONTHS_PER_YEAR) / 3) + 1);
+ else if (interval->day > 0 ||
+ (interval->day == 0 && interval->time >= 0))
+ intresult = 1;
+ else
+ intresult = -1;
break;
case DTK_YEAR:
diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
index 51ae010c7b..a6009f8e6a 100644
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -1834,6 +1834,7 @@ SELECT f1,
EXTRACT(MINUTE FROM f1) AS MINUTE,
EXTRACT(HOUR FROM f1) AS HOUR,
EXTRACT(DAY FROM f1) AS DAY,
+ EXTRACT(WEEK FROM f1) AS WEEK,
EXTRACT(MONTH FROM f1) AS MONTH,
EXTRACT(QUARTER FROM f1) AS QUARTER,
EXTRACT(YEAR FROM f1) AS YEAR,
@@ -1842,20 +1843,52 @@ SELECT f1,
EXTRACT(MILLENNIUM FROM f1) AS MILLENNIUM,
EXTRACT(EPOCH FROM f1) AS EPOCH
FROM INTERVAL_TBL;
- f1 | microsecond | millisecond | second | minute | hour | day | month | quarter | year | decade | century | millennium | epoch
--------------------------------+-------------+-------------+------------+--------+-----------+-----------+-------+---------+-----------+-----------+-----------+------------+-------------------
- @ 1 min | 0 | 0.000 | 0.000000 | 1 | 0 | 0 | 0 | 1 | 0 | 0 | 0 | 0 | 60.000000
- @ 5 hours | 0 | 0.000 | 0.000000 | 0 | 5 | 0 | 0 | 1 | 0 | 0 | 0 | 0 | 18000.000000
- @ 10 days | 0 | 0.000 | 0.000000 | 0 | 0 | 10 | 0 | 1 | 0 | 0 | 0 | 0 | 864000.000000
- @ 34 years | 0 | 0.000 | 0.000000 | 0 | 0 | 0 | 0 | 1 | 34 | 3 | 0 | 0 | 1072958400.000000
- @ 3 mons | 0 | 0.000 | 0.000000 | 0 | 0 | 0 | 3 | 2 | 0 | 0 | 0 | 0 | 7776000.000000
- @ 14 secs ago | -14000000 | -14000.000 | -14.000000 | 0 | 0 | 0 | 0 | 1 | 0 | 0 | 0 | 0 | -14.000000
- @ 1 day 2 hours 3 mins 4 secs | 4000000 | 4000.000 | 4.000000 | 3 | 2 | 1 | 0 | 1 | 0 | 0 | 0 | 0 | 93784.000000
- @ 6 years | 0 | 0.000 | 0.000000 | 0 | 0 | 0 | 0 | 1 | 6 | 0 | 0 | 0 | 189345600.000000
- @ 5 mons | 0 | 0.000 | 0.000000 | 0 | 0 | 0 | 5 | 2 | 0 | 0 | 0 | 0 | 12960000.000000
- @ 5 mons 12 hours | 0 | 0.000 | 0.000000 | 0 | 12 | 0 | 5 | 2 | 0 | 0 | 0 | 0 | 13003200.000000
- infinity | | | | | Infinity | Infinity | | | Infinity | Infinity | Infinity | Infinity | Infinity
- -infinity | | | | | -Infinity | -Infinity | | | -Infinity | -Infinity | -Infinity | -Infinity | -Infinity
+ f1 | microsecond | millisecond | second | minute | hour | day | week | month | quarter | year | decade | century | millennium | epoch
+-------------------------------+-------------+-------------+------------+--------+-----------+-----------+------+-------+---------+-----------+-----------+-----------+------------+-------------------
+ @ 1 min | 0 | 0.000 | 0.000000 | 1 | 0 | 0 | 0 | 0 | 1 | 0 | 0 | 0 | 0 | 60.000000
+ @ 5 hours | 0 | 0.000 | 0.000000 | 0 | 5 | 0 | 0 | 0 | 1 | 0 | 0 | 0 | 0 | 18000.000000
+ @ 10 days | 0 | 0.000 | 0.000000 | 0 | 0 | 10 | 1 | 0 | 1 | 0 | 0 | 0 | 0 | 864000.000000
+ @ 34 years | 0 | 0.000 | 0.000000 | 0 | 0 | 0 | 0 | 0 | 1 | 34 | 3 | 0 | 0 | 1072958400.000000
+ @ 3 mons | 0 | 0.000 | 0.000000 | 0 | 0 | 0 | 0 | 3 | 2 | 0 | 0 | 0 | 0 | 7776000.000000
+ @ 14 secs ago | -14000000 | -14000.000 | -14.000000 | 0 | 0 | 0 | 0 | 0 | -1 | 0 | 0 | 0 | 0 | -14.000000
+ @ 1 day 2 hours 3 mins 4 secs | 4000000 | 4000.000 | 4.000000 | 3 | 2 | 1 | 0 | 0 | 1 | 0 | 0 | 0 | 0 | 93784.000000
+ @ 6 years | 0 | 0.000 | 0.000000 | 0 | 0 | 0 | 0 | 0 | 1 | 6 | 0 | 0 | 0 | 189345600.000000
+ @ 5 mons | 0 | 0.000 | 0.000000 | 0 | 0 | 0 | 0 | 5 | 2 | 0 | 0 | 0 | 0 | 12960000.000000
+ @ 5 mons 12 hours | 0 | 0.000 | 0.000000 | 0 | 12 | 0 | 0 | 5 | 2 | 0 | 0 | 0 | 0 | 13003200.000000
+ infinity | | | | | Infinity | Infinity | | | | Infinity | Infinity | Infinity | Infinity | Infinity
+ -infinity | | | | | -Infinity | -Infinity | | | | -Infinity | -Infinity | -Infinity | -Infinity | -Infinity
+(12 rows)
+
+SELECT -f1,
+ EXTRACT(MICROSECOND FROM -f1) AS MICROSECOND,
+ EXTRACT(MILLISECOND FROM -f1) AS MILLISECOND,
+ EXTRACT(SECOND FROM -f1) AS SECOND,
+ EXTRACT(MINUTE FROM -f1) AS MINUTE,
+ EXTRACT(HOUR FROM -f1) AS HOUR,
+ EXTRACT(DAY FROM -f1) AS DAY,
+ EXTRACT(WEEK FROM -f1) AS WEEK,
+ EXTRACT(MONTH FROM -f1) AS MONTH,
+ EXTRACT(QUARTER FROM -f1) AS QUARTER,
+ EXTRACT(YEAR FROM -f1) AS YEAR,
+ EXTRACT(DECADE FROM -f1) AS DECADE,
+ EXTRACT(CENTURY FROM -f1) AS CENTURY,
+ EXTRACT(MILLENNIUM FROM -f1) AS MILLENNIUM,
+ EXTRACT(EPOCH FROM -f1) AS EPOCH
+ FROM INTERVAL_TBL;
+ ?column? | microsecond | millisecond | second | minute | hour | day | week | month | quarter | year | decade | century | millennium | epoch
+-----------------------------------+-------------+-------------+-----------+--------+-----------+-----------+------+-------+---------+-----------+-----------+-----------+------------+--------------------
+ @ 1 min ago | 0 | 0.000 | 0.000000 | -1 | 0 | 0 | 0 | 0 | -1 | 0 | 0 | 0 | 0 | -60.000000
+ @ 5 hours ago | 0 | 0.000 | 0.000000 | 0 | -5 | 0 | 0 | 0 | -1 | 0 | 0 | 0 | 0 | -18000.000000
+ @ 10 days ago | 0 | 0.000 | 0.000000 | 0 | 0 | -10 | -1 | 0 | -1 | 0 | 0 | 0 | 0 | -864000.000000
+ @ 34 years ago | 0 | 0.000 | 0.000000 | 0 | 0 | 0 | 0 | 0 | -1 | -34 | -3 | 0 | 0 | -1072958400.000000
+ @ 3 mons ago | 0 | 0.000 | 0.000000 | 0 | 0 | 0 | 0 | -3 | -2 | 0 | 0 | 0 | 0 | -7776000.000000
+ @ 14 secs | 14000000 | 14000.000 | 14.000000 | 0 | 0 | 0 | 0 | 0 | 1 | 0 | 0 | 0 | 0 | 14.000000
+ @ 1 day 2 hours 3 mins 4 secs ago | -4000000 | -4000.000 | -4.000000 | -3 | -2 | -1 | 0 | 0 | -1 | 0 | 0 | 0 | 0 | -93784.000000
+ @ 6 years ago | 0 | 0.000 | 0.000000 | 0 | 0 | 0 | 0 | 0 | -1 | -6 | 0 | 0 | 0 | -189345600.000000
+ @ 5 mons ago | 0 | 0.000 | 0.000000 | 0 | 0 | 0 | 0 | -5 | -2 | 0 | 0 | 0 | 0 | -12960000.000000
+ @ 5 mons 12 hours ago | 0 | 0.000 | 0.000000 | 0 | -12 | 0 | 0 | -5 | -2 | 0 | 0 | 0 | 0 | -13003200.000000
+ -infinity | | | | | -Infinity | -Infinity | | | | -Infinity | -Infinity | -Infinity | -Infinity | -Infinity
+ infinity | | | | | Infinity | Infinity | | | | Infinity | Infinity | Infinity | Infinity | Infinity
(12 rows)
SELECT EXTRACT(FORTNIGHT FROM INTERVAL '2 days'); -- error
diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql
index fbf6e064d6..55054ae65d 100644
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -592,6 +592,7 @@ SELECT f1,
EXTRACT(MINUTE FROM f1) AS MINUTE,
EXTRACT(HOUR FROM f1) AS HOUR,
EXTRACT(DAY FROM f1) AS DAY,
+ EXTRACT(WEEK FROM f1) AS WEEK,
EXTRACT(MONTH FROM f1) AS MONTH,
EXTRACT(QUARTER FROM f1) AS QUARTER,
EXTRACT(YEAR FROM f1) AS YEAR,
@@ -601,6 +602,23 @@ SELECT f1,
EXTRACT(EPOCH FROM f1) AS EPOCH
FROM INTERVAL_TBL;
+SELECT -f1,
+ EXTRACT(MICROSECOND FROM -f1) AS MICROSECOND,
+ EXTRACT(MILLISECOND FROM -f1) AS MILLISECOND,
+ EXTRACT(SECOND FROM -f1) AS SECOND,
+ EXTRACT(MINUTE FROM -f1) AS MINUTE,
+ EXTRACT(HOUR FROM -f1) AS HOUR,
+ EXTRACT(DAY FROM -f1) AS DAY,
+ EXTRACT(WEEK FROM -f1) AS WEEK,
+ EXTRACT(MONTH FROM -f1) AS MONTH,
+ EXTRACT(QUARTER FROM -f1) AS QUARTER,
+ EXTRACT(YEAR FROM -f1) AS YEAR,
+ EXTRACT(DECADE FROM -f1) AS DECADE,
+ EXTRACT(CENTURY FROM -f1) AS CENTURY,
+ EXTRACT(MILLENNIUM FROM -f1) AS MILLENNIUM,
+ EXTRACT(EPOCH FROM -f1) AS EPOCH
+ FROM INTERVAL_TBL;
+
SELECT EXTRACT(FORTNIGHT FROM INTERVAL '2 days'); -- error
SELECT EXTRACT(TIMEZONE FROM INTERVAL '2 days'); -- error
=
view thread (31+ messages) latest in thread
Message-ID: <1079670.1720458208@sss.pgh.pa.us>
Permalink: ../1079670.1720458208@sss.pgh.pa.us/
Also on: postgresql.org/message-id/1079670.1720458208@sss.pgh.pa.us
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, jian.universality@gmail.com, folarte@peoplecall.com, work.michael.2956@gmail.com, peter.eisentraut@enterprisedb.com
Subject: Re: BUG #18348: Inconsistency with EXTRACT([field] from INTERVAL);
In-Reply-To: <1079670.1720458208@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox