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: Tue, 07 May 2024 17:27:08 -0400
Message-ID: <166456.1715117228@sss.pgh.pa.us> (raw )
In-Reply-To: <CACJufxFOjGpAQJ5EJ0=a1geERgdHW77SEdRyK8A_qwB3CVdwhQ@mail.gmail.com >
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 >
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
Attachments:
[text/x-diff] v2-interval-week-and-quarter-fixes.patch (13.5K, ../166456.1715117228@sss.pgh.pa.us/2-v2-interval-week-and-quarter-fixes.patch)
download | inline diff:
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 17c44bc338..e7792fbae1 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 e4715605a2..3e65046117 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,27 @@ 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 (interval->month >= 0)
+ intresult = (tm->tm_mon / 3) + 1;
+ else
+ intresult = -(((-interval->month % MONTHS_PER_YEAR) / 3) + 1);
break;
case DTK_YEAR:
diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
index 51ae010c7b..e5d919d0cf 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: <166456.1715117228@sss.pgh.pa.us>
Permalink: ../166456.1715117228@sss.pgh.pa.us/
Also on: postgresql.org/message-id/166456.1715117228@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: <166456.1715117228@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