Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lcHVz-0007q5-HG for pgsql-bugs@arkaria.postgresql.org; Fri, 30 Apr 2021 01:00:19 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1lcHVx-0002SP-D3 for pgsql-bugs@arkaria.postgresql.org; Fri, 30 Apr 2021 01:00:17 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lcHVx-0002SI-5k for pgsql-bugs@lists.postgresql.org; Fri, 30 Apr 2021 01:00:17 +0000 Received: from momjian.us ([72.94.173.45]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lcHVo-00053l-Eb for pgsql-bugs@lists.postgresql.org; Fri, 30 Apr 2021 01:00:16 +0000 Received: from bruce by momjian.us with local (Exim 4.92) (envelope-from ) id 1lcHVk-0002xA-Bn; Thu, 29 Apr 2021 21:00:04 -0400 Date: Thu, 29 Apr 2021 21:00:04 -0400 From: Bruce Momjian To: Noah Misch Cc: Tom Lane , neverov.max@gmail.com, pgsql-bugs@lists.postgresql.org Subject: Re: BUG #16939: Plural interval for negative singular Message-ID: <20210430010004.GA23068@momjian.us> 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> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="Q68bSM7Ycu6FN28Q" Content-Disposition: inline In-Reply-To: <20210428054415.GA2493304@rfd.leadboat.com> User-Agent: Mutt/1.10.1 (2018-07-13) List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --Q68bSM7Ycu6FN28Q Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 https://momjian.us EDB https://enterprisedb.com If only the physical world exists, free will is an illusion. --Q68bSM7Ycu6FN28Q Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="negative.diff" diff --git a/contrib/dblink/expected/dblink.out b/contrib/dblink/expected/dblink.out index 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.c index 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.c index 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.c index 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("\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.out index 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 --Q68bSM7Ycu6FN28Q--