From: Bruce Momjian <bruce@momjian.us>
To: Noah Misch <noah@leadboat.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: neverov.max@gmail.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16939: Plural interval for negative singular
Date: Thu, 29 Apr 2021 21:00:04 -0400
Message-ID: <20210430010004.GA23068@momjian.us> (raw)
In-Reply-To: <20210428054415.GA2493304@rfd.leadboat.com>
References: <20210323160336.GE579@momjian.us>
<20210425115726.GA2353095@rfd.leadboat.com>
<20210426163116.GR7629@momjian.us>
<2266336.1619455534@sss.pgh.pa.us>
<20210426165435.GT7629@momjian.us>
<2267469.1619456564@sss.pgh.pa.us>
<20210426170616.GV7629@momjian.us>
<20210427061329.GB2435607@rfd.leadboat.com>
<20210427201019.GY7629@momjian.us>
<20210428054415.GA2493304@rfd.leadboat.com>
On Tue, Apr 27, 2021 at 10:44:15PM -0700, Noah Misch wrote:
> On Tue, Apr 27, 2021 at 04:10:19PM -0400, Bruce Momjian wrote:
> > Yes, I found the same like you did. Are you saying we should remove the
> > abs() calls here so we are consistent?
>
> No, I advise just reverting 5da9868. The output funcs should keep doing what
> they did in v13, inconsistency and all.
>
> I was asking the above question to see which of singular or plural was more
> common before $SUBJECT's commit. I count two examples of singular and seven
> examples of plural (one in EVALUATE_MESSAGE_PLURAL(), six modified in commit
> 5da9868). Given that, ...
>
> On Mon, Apr 26, 2021 at 01:06:16PM -0400, Bruce Momjian wrote:
> > Well, we should then add a comment that this is inconsistent but should
> > not be changed.
>
> ... adding comments to the two places that use abs() to achieve singular "-1"
> sounds fine.
OK, first, I never realized that saying -1 plural was valid. Second,
the two places you showed are actually using abs() properly since
negative values are shown with an "ago" suffix, not a negative sign:
SET intervalstyle = postgres_verbose;
SELECT '2020-01-01 01:03:00'::timestamptz - '2020-01-01 02:02:01'::timestamptz;
?column?
---------------------
@ 59 mins 1 sec ago
(1 row)
Attached is a patch which reverts 9ee7d533da and 5da9868ed9, and adds a
comment about the "ago" usage. The good news is that we were already
consistent in using plural -1. :-)
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
If only the physical world exists, free will is an illusion.
Attachments:negative.diff (text/x-diff, 5.7K) downloaddiff --git a/contrib/dblink/expected/dblink.out b/contrib/dblink/expected/dblink.outindex 33ceaddd89..6516d4f131 100644--- a/contrib/dblink/expected/dblink.out+++ b/contrib/dblink/expected/dblink.out@@ -1099,9 +1099,9 @@ SELECT *
FROM dblink('myconn',
'SELECT * FROM (VALUES (''-1 2:03:04'')) i')
AS i(i interval);
- i -------------------- -1 day -02:03:04+ i +-------------------+ -1 days -02:03:04
(1 row)
-- Try swapping to another format to ensure the GUCs are tracked
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.cindex 889077f55c..1d8b393966 100644--- a/src/backend/utils/adt/datetime.c+++ b/src/backend/utils/adt/datetime.c@@ -4190,7 +4190,7 @@ AddPostgresIntPart(char *cp, int value, const char *units,
(*is_before && value > 0) ? "+" : "",
value,
units,
- (abs(value) != 1) ? "s" : "");+ (value != 1) ? "s" : "");
/*
* Each nonzero field sets is_before for (only) the next one. This is a
@@ -4216,7 +4216,7 @@ AddVerboseIntPart(char *cp, int value, const char *units,
}
else if (*is_before)
value = -value;
- sprintf(cp, " %d %s%s", value, units, (abs(value) == 1) ? "" : "s");+ sprintf(cp, " %d %s%s", value, units, (value == 1) ? "" : "s");
*is_zero = false;
return cp + strlen(cp);
}
@@ -4414,6 +4414,7 @@ EncodeInterval(struct pg_tm *tm, fsec_t fsec, int style, char *str)
else if (is_before)
*cp++ = '-';
cp = AppendSeconds(cp, sec, fsec, MAX_INTERVAL_PRECISION, false);
+ /* We output "ago", not negatives, so use abs() here. */
sprintf(cp, " sec%s",
(abs(sec) != 1 || fsec != 0) ? "s" : "");
is_zero = false;
diff --git a/src/interfaces/ecpg/pgtypeslib/interval.c b/src/interfaces/ecpg/pgtypeslib/interval.cindex 4245016c8e..5ccc19c43d 100644--- a/src/interfaces/ecpg/pgtypeslib/interval.c+++ b/src/interfaces/ecpg/pgtypeslib/interval.c@@ -694,7 +694,7 @@ AddVerboseIntPart(char *cp, int value, const char *units,
}
else if (*is_before)
value = -value;
- sprintf(cp, " %d %s%s", value, units, (abs(value) == 1) ? "" : "s");+ sprintf(cp, " %d %s%s", value, units, (value == 1) ? "" : "s");
*is_zero = false;
return cp + strlen(cp);
}
@@ -711,7 +711,7 @@ AddPostgresIntPart(char *cp, int value, const char *units,
(*is_before && value > 0) ? "+" : "",
value,
units,
- (abs(value) != 1) ? "s" : "");+ (value != 1) ? "s" : "");
/*
* Each nonzero field sets is_before for (only) the next one. This is a
@@ -924,6 +924,7 @@ EncodeInterval(struct /* pg_ */ tm *tm, fsec_t fsec, int style, char *str)
*cp++ = '-';
AppendSeconds(cp, sec, fsec, MAX_INTERVAL_PRECISION, false);
cp += strlen(cp);
+ /* We output "ago", not negatives, so use abs() here. */
sprintf(cp, " sec%s",
(abs(sec) != 1 || fsec != 0) ? "s" : "");
is_zero = false;
diff --git a/src/interfaces/libpq/fe-print.c b/src/interfaces/libpq/fe-print.cindex af19b3c0a3..94219b1825 100644--- a/src/interfaces/libpq/fe-print.c+++ b/src/interfaces/libpq/fe-print.c@@ -303,7 +303,7 @@ PQprint(FILE *fout, const PGresult *res, const PQprintOpt *po)
}
if (po->header && !po->html3)
fprintf(fout, "(%d row%s)\n\n", PQntuples(res),
- (abs(PQntuples(res)) == 1) ? "" : "s");+ (PQntuples(res) == 1) ? "" : "s");
if (po->html3 && !po->expanded)
fputs("</table>\n", fout);
free(fieldMax);
@@ -662,7 +662,7 @@ PQdisplayTuples(const PGresult *res,
if (!quiet)
fprintf(fp, "\nQuery returned %d row%s.\n", PQntuples(res),
- (abs(PQntuples(res)) == 1) ? "" : "s");+ (PQntuples(res) == 1) ? "" : "s");
fflush(fp);
diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.outindex 0191949137..e4b1246f45 100644--- a/src/test/regress/expected/interval.out+++ b/src/test/regress/expected/interval.out@@ -23,15 +23,15 @@ SELECT INTERVAL '-08:00' AS "Eight hours";
(1 row)
SELECT INTERVAL '-1 +02:03' AS "22 hours ago...";
- 22 hours ago... -------------------- -1 day +02:03:00+ 22 hours ago... +-------------------+ -1 days +02:03:00
(1 row)
SELECT INTERVAL '-1 days +02:03' AS "22 hours ago...";
- 22 hours ago... -------------------- -1 day +02:03:00+ 22 hours ago... +-------------------+ -1 days +02:03:00
(1 row)
SELECT INTERVAL '1.5 weeks' AS "Ten days twelve hours";
@@ -288,7 +288,7 @@ FROM INTERVAL_MULDIV_TBL;
product
------------------------------------
1 year 12 days 122:24:00
- -1 year -12 days +93:36:00+ -1 years -12 days +93:36:00
-3 days -14:24:00
2 mons 13 days 01:22:28.8
-10 mons +120 days 37:28:21.6567
@@ -317,7 +317,7 @@ FROM INTERVAL_MULDIV_TBL;
----------------------------------
4 mons 4 days 40:48:00
-4 mons -4 days +31:12:00
- -1 day -04:48:00+ -1 days -04:48:00
25 days -15:32:30.4
-3 mons +30 days 12:29:27.2189
12 days
@@ -785,9 +785,9 @@ SELECT interval '+1 -1:00:00',
interval '-1 +1:00:00',
interval '+1-2 -3 +4:05:06.789',
interval '-1-2 +3 -4:05:06.789';
- interval | interval | interval | interval ------------------+------------------+-------------------------------------+---------------------------------------- 1 day -01:00:00 | -1 day +01:00:00 | 1 year 2 mons -3 days +04:05:06.789 | -1 year -2 mons +3 days -04:05:06.789+ interval | interval | interval | interval +-----------------+-------------------+-------------------------------------+----------------------------------------+ 1 day -01:00:00 | -1 days +01:00:00 | 1 year 2 mons -3 days +04:05:06.789 | -1 years -2 mons +3 days -04:05:06.789
(1 row)
-- test output of couple non-standard interval values in the sql style
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: bruce@momjian.us, noah@leadboat.com, tgl@sss.pgh.pa.us, neverov.max@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16939: Plural interval for negative singular
In-Reply-To: <20210430010004.GA23068@momjian.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