pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
BUG #16939: Plural interval for negative singular
20+ messages / 6 participants
[nested] [flat]

Commits from this thread:
  2021-03-30 5da9868ed9  In messages, use singular nouns for -1, like we do for +1.
* BUG #16939: Plural interval for negative singular
@ 2021-03-23 05:21 PG Bug reporting form <noreply@postgresql.org>
  2021-03-23 16:03 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  0 siblings, 1 reply; 20+ messages in thread

From: PG Bug reporting form @ 2021-03-23 05:21 UTC (permalink / raw)
  To: pgsql-bugs@lists.postgresql.org; +Cc: neverov.max@gmail.com

The following bug has been logged on the website:

Bug reference:      16939
Logged by:          Max Neverov
Email address:      neverov.max@gmail.com
PostgreSQL version: 13.2
Operating system:   alpine
Description:        

Execute the query:
postgres=# set intervalstyle='postgres';
SET
postgres=#  select interval '-1 year -1 day';
     interval
------------------
 -1 years -1 days
(1 row)

Expected output:
-1 year -1 day

The code
(https://github.com/postgres/postgres/blob/master/src/backend/utils/adt/datetime.c#L4193)
pluralizes a time unit if the value is not 1, should check both -1 and 1.



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: BUG #16939: Plural interval for negative singular
  2021-03-23 05:21 BUG #16939: Plural interval for negative singular PG Bug reporting form <noreply@postgresql.org>
@ 2021-03-23 16:03 ` Bruce Momjian <bruce@momjian.us>
  2021-03-30 22:35   ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-25 11:57   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  0 siblings, 2 replies; 20+ messages in thread

From: Bruce Momjian @ 2021-03-23 16:03 UTC (permalink / raw)
  To: neverov.max@gmail.com; pgsql-bugs@lists.postgresql.org

On Tue, Mar 23, 2021 at 05:21:30AM +0000, PG Bug reporting form wrote:
> The following bug has been logged on the website:
> 
> Bug reference:      16939
> Logged by:          Max Neverov
> Email address:      neverov.max@gmail.com
> PostgreSQL version: 13.2
> Operating system:   alpine
> Description:        
> 
> Execute the query:
> postgres=# set intervalstyle='postgres';
> SET
> postgres=#  select interval '-1 year -1 day';
>      interval
> ------------------
>  -1 years -1 days
> (1 row)
> 
> Expected output:
> -1 year -1 day
> 
> The code
> (https://github.com/postgres/postgres/blob/master/src/backend/utils/adt/datetime.c#L4193)
> pluralizes a time unit if the value is not 1, should check both -1 and 1.
> 

Wow, interesting.  Seems there were a few places in the code that
handled this properly, and several that didn't.  The attached patch
fixes these and updates the regression tests.  I will apply this for PG
14 in the next few days.  This is too likely to break something to
backpatch.  Thanks for the report.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.

Attachments:

  plural.diff (text/x-diff, 0B) download

^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: BUG #16939: Plural interval for negative singular
  2021-03-23 05:21 BUG #16939: Plural interval for negative singular PG Bug reporting form <noreply@postgresql.org>
  2021-03-23 16:03 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
@ 2021-03-30 22:35   ` Bruce Momjian <bruce@momjian.us>
  1 sibling, 0 replies; 20+ messages in thread

From: Bruce Momjian @ 2021-03-30 22:35 UTC (permalink / raw)
  To: neverov.max@gmail.com; pgsql-bugs@lists.postgresql.org

On Tue, Mar 23, 2021 at 12:03:36PM -0400, Bruce Momjian wrote:
> On Tue, Mar 23, 2021 at 05:21:30AM +0000, PG Bug reporting form wrote:
> > The following bug has been logged on the website:
> > 
> > Bug reference:      16939
> > Logged by:          Max Neverov
> > Email address:      neverov.max@gmail.com
> > PostgreSQL version: 13.2
> > Operating system:   alpine
> > Description:        
> > 
> > Execute the query:
> > postgres=# set intervalstyle='postgres';
> > SET
> > postgres=#  select interval '-1 year -1 day';
> >      interval
> > ------------------
> >  -1 years -1 days
> > (1 row)
> > 
> > Expected output:
> > -1 year -1 day
> > 
> > The code
> > (https://github.com/postgres/postgres/blob/master/src/backend/utils/adt/datetime.c#L4193)
> > pluralizes a time unit if the value is not 1, should check both -1 and 1.
> > 
> 
> Wow, interesting.  Seems there were a few places in the code that
> handled this properly, and several that didn't.  The attached patch
> fixes these and updates the regression tests.  I will apply this for PG
> 14 in the next few days.  This is too likely to break something to
> backpatch.  Thanks for the report.

Patch applied and will appear in PG 14.  Thanks for the report.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: BUG #16939: Plural interval for negative singular
  2021-03-23 05:21 BUG #16939: Plural interval for negative singular PG Bug reporting form <noreply@postgresql.org>
  2021-03-23 16:03 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
@ 2021-04-25 11:57   ` Noah Misch <noah@leadboat.com>
  2021-04-26 16:31     ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  1 sibling, 1 reply; 20+ messages in thread

From: Noah Misch @ 2021-04-25 11:57 UTC (permalink / raw)
  To: Bruce Momjian <bruce@momjian.us>; +Cc: neverov.max@gmail.com; pgsql-bugs@lists.postgresql.org

On Tue, Mar 23, 2021 at 12:03:36PM -0400, Bruce Momjian wrote:
> On Tue, Mar 23, 2021 at 05:21:30AM +0000, PG Bug reporting form wrote:
> > postgres=# set intervalstyle='postgres';
> > SET
> > postgres=#  select interval '-1 year -1 day';
> >      interval
> > ------------------
> >  -1 years -1 days
> > (1 row)
> > 
> > Expected output:
> > -1 year -1 day
> > 
> > The code
> > (https://github.com/postgres/postgres/blob/master/src/backend/utils/adt/datetime.c#L4193)
> > pluralizes a time unit if the value is not 1, should check both -1 and 1.



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: BUG #16939: Plural interval for negative singular
  2021-03-23 05:21 BUG #16939: Plural interval for negative singular PG Bug reporting form <noreply@postgresql.org>
  2021-03-23 16:03 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-25 11:57   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
@ 2021-04-26 16:31     ` Bruce Momjian <bruce@momjian.us>
  2021-04-26 16:45       ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 20+ messages in thread

From: Bruce Momjian @ 2021-04-26 16:31 UTC (permalink / raw)
  To: Noah Misch <noah@leadboat.com>; +Cc: neverov.max@gmail.com; pgsql-bugs@lists.postgresql.org

On Sun, Apr 25, 2021 at 04:57:26AM -0700, Noah Misch wrote:
> On Tue, Mar 23, 2021 at 12:03:36PM -0400, Bruce Momjian wrote:
> > On Tue, Mar 23, 2021 at 05:21:30AM +0000, PG Bug reporting form wrote:
> > > postgres=# set intervalstyle='postgres';
> > > SET
> > > postgres=#  select interval '-1 year -1 day';
> > >      interval
> > > ------------------
> > >  -1 years -1 days
> > > (1 row)
> > > 
> > > Expected output:
> > > -1 year -1 day
> > > 
> > > The code
> > > (https://github.com/postgres/postgres/blob/master/src/backend/utils/adt/datetime.c#L4193)
> > > pluralizes a time unit if the value is not 1, should check both -1 and 1.
> 
> >From the first several links at
> https://www.google.com/search?q=negative+one+singular+plural the choice is a
> matter of dispute among English grammar commentators.  Singular "-1" is
> popular, and plural "-1" is also popular.
> 
> > Wow, interesting.  Seems there were a few places in the code that
> > handled this properly, and several that didn't.  The attached patch
> > fixes these and updates the regression tests.  I will apply this for PG
> > 14 in the next few days.  This is too likely to break something to
> > backpatch.  Thanks for the report.
> 
> Let's not change from one popular spelling to another when doing so creates a
> compatibility hazard.  That is to say, I think PostgreSQL would be better with
> this patch reverted.
> 
> If we did want to standardize on singular for -1, EVALUATE_MESSAGE_PLURAL()
> would be a key bit of code to change.

Oh, good point.  I think we should just pick one and be consistent --- I
don't care which we choose.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: BUG #16939: Plural interval for negative singular
  2021-03-23 05:21 BUG #16939: Plural interval for negative singular PG Bug reporting form <noreply@postgresql.org>
  2021-03-23 16:03 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-25 11:57   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-26 16:31     ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
@ 2021-04-26 16:45       ` Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 16:54         ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  0 siblings, 1 reply; 20+ messages in thread

From: Tom Lane @ 2021-04-26 16:45 UTC (permalink / raw)
  To: Bruce Momjian <bruce@momjian.us>; +Cc: Noah Misch <noah@leadboat.com>; neverov.max@gmail.com; pgsql-bugs@lists.postgresql.org

Bruce Momjian <bruce@momjian.us> writes:
> On Sun, Apr 25, 2021 at 04:57:26AM -0700, Noah Misch wrote:
>> Let's not change from one popular spelling to another when doing so creates a
>> compatibility hazard.  That is to say, I think PostgreSQL would be better with
>> this patch reverted.
>> 
>> If we did want to standardize on singular for -1, EVALUATE_MESSAGE_PLURAL()
>> would be a key bit of code to change.

> Oh, good point.  I think we should just pick one and be consistent --- I
> don't care which we choose.

I agree with Noah's opinion that we should stick to the historical
behavior in the interval I/O functions.  There is not enough solidity
in the "this is grammatically wrong" argument to justify taking any
risk of application breakage, and it seems like there is some risk of
that there.

For the sorts of human-readable messages that EVALUATE_MESSAGE_PLURAL
tends to be used for, I don't think there's a reason to worry that
we might break applications if we change it.  So I don't have a
strong opinion about what to do there.  Still, by the same token
that the grammatical argument is weak, I lean towards not spending
effort on changing it.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: BUG #16939: Plural interval for negative singular
  2021-03-23 05:21 BUG #16939: Plural interval for negative singular PG Bug reporting form <noreply@postgresql.org>
  2021-03-23 16:03 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-25 11:57   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-26 16:31     ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 16:45       ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-04-26 16:54         ` Bruce Momjian <bruce@momjian.us>
  2021-04-26 17:02           ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 20+ messages in thread

From: Bruce Momjian @ 2021-04-26 16:54 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Noah Misch <noah@leadboat.com>; neverov.max@gmail.com; pgsql-bugs@lists.postgresql.org

On Mon, Apr 26, 2021 at 12:45:34PM -0400, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > On Sun, Apr 25, 2021 at 04:57:26AM -0700, Noah Misch wrote:
> >> Let's not change from one popular spelling to another when doing so creates a
> >> compatibility hazard.  That is to say, I think PostgreSQL would be better with
> >> this patch reverted.
> >> 
> >> If we did want to standardize on singular for -1, EVALUATE_MESSAGE_PLURAL()
> >> would be a key bit of code to change.
> 
> > Oh, good point.  I think we should just pick one and be consistent --- I
> > don't care which we choose.
> 
> I agree with Noah's opinion that we should stick to the historical
> behavior in the interval I/O functions.  There is not enough solidity
> in the "this is grammatically wrong" argument to justify taking any
> risk of application breakage, and it seems like there is some risk of
> that there.
> 
> For the sorts of human-readable messages that EVALUATE_MESSAGE_PLURAL
> tends to be used for, I don't think there's a reason to worry that
> we might break applications if we change it.  So I don't have a
> strong opinion about what to do there.  Still, by the same token
> that the grammatical argument is weak, I lean towards not spending
> effort on changing it.

Are you saying we should revert the patch and leave the plurals
inconsistent in different places?  We were already expressing -1 as
singular in most of the places I saw.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: BUG #16939: Plural interval for negative singular
  2021-03-23 05:21 BUG #16939: Plural interval for negative singular PG Bug reporting form <noreply@postgresql.org>
  2021-03-23 16:03 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-25 11:57   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-26 16:31     ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 16:45       ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 16:54         ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
@ 2021-04-26 17:02           ` Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 17:06             ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-27 22:50             ` Re: BUG #16939: Plural interval for negative singular Alvaro Herrera <alvherre@alvh.no-ip.org>
  0 siblings, 2 replies; 20+ messages in thread

From: Tom Lane @ 2021-04-26 17:02 UTC (permalink / raw)
  To: Bruce Momjian <bruce@momjian.us>; +Cc: Noah Misch <noah@leadboat.com>; neverov.max@gmail.com; pgsql-bugs@lists.postgresql.org

Bruce Momjian <bruce@momjian.us> writes:
> On Mon, Apr 26, 2021 at 12:45:34PM -0400, Tom Lane wrote:
>> I agree with Noah's opinion that we should stick to the historical
>> behavior in the interval I/O functions.  There is not enough solidity
>> in the "this is grammatically wrong" argument to justify taking any
>> risk of application breakage, and it seems like there is some risk of
>> that there.

> Are you saying we should revert the patch and leave the plurals
> inconsistent in different places?

As far as the changes in datetime.c and interval.c are concerned,
yes.  I don't care too much about what you did in fe-print.c,
although TBH that case should be unreachable shouldn't it?
When would PQntuples() return -1?

(I shy gently away from the fact that that fe-print.c code is
relentlessly untranslatable.)

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: BUG #16939: Plural interval for negative singular
  2021-03-23 05:21 BUG #16939: Plural interval for negative singular PG Bug reporting form <noreply@postgresql.org>
  2021-03-23 16:03 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-25 11:57   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-26 16:31     ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 16:45       ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 16:54         ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 17:02           ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-04-26 17:06             ` Bruce Momjian <bruce@momjian.us>
  2021-04-27 06:13               ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  1 sibling, 1 reply; 20+ messages in thread

From: Bruce Momjian @ 2021-04-26 17:06 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Noah Misch <noah@leadboat.com>; neverov.max@gmail.com; pgsql-bugs@lists.postgresql.org

On Mon, Apr 26, 2021 at 01:02:44PM -0400, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > On Mon, Apr 26, 2021 at 12:45:34PM -0400, Tom Lane wrote:
> >> I agree with Noah's opinion that we should stick to the historical
> >> behavior in the interval I/O functions.  There is not enough solidity
> >> in the "this is grammatically wrong" argument to justify taking any
> >> risk of application breakage, and it seems like there is some risk of
> >> that there.
> 
> > Are you saying we should revert the patch and leave the plurals
> > inconsistent in different places?
> 
> As far as the changes in datetime.c and interval.c are concerned,
> yes.  I don't care too much about what you did in fe-print.c,

Well, we should then add a comment that this is inconsistent but should
not be changed.

> although TBH that case should be unreachable shouldn't it?
> When would PQntuples() return -1?

I changed that just for consistency with other calls, in case it is ever
copied somewhere else.

> (I shy gently away from the fact that that fe-print.c code is
> relentlessly untranslatable.)

Again, if we revert, we should document is via a C comment.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: BUG #16939: Plural interval for negative singular
  2021-03-23 05:21 BUG #16939: Plural interval for negative singular PG Bug reporting form <noreply@postgresql.org>
  2021-03-23 16:03 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-25 11:57   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-26 16:31     ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 16:45       ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 16:54         ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 17:02           ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 17:06             ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
@ 2021-04-27 06:13               ` Noah Misch <noah@leadboat.com>
  2021-04-27 20:10                 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  0 siblings, 1 reply; 20+ messages in thread

From: Noah Misch @ 2021-04-27 06:13 UTC (permalink / raw)
  To: Bruce Momjian <bruce@momjian.us>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; neverov.max@gmail.com; pgsql-bugs@lists.postgresql.org

On Mon, Apr 26, 2021 at 01:06:16PM -0400, Bruce Momjian wrote:
> On Mon, Apr 26, 2021 at 01:02:44PM -0400, Tom Lane wrote:
> > Bruce Momjian <bruce@momjian.us> writes:
> > > On Mon, Apr 26, 2021 at 12:45:34PM -0400, Tom Lane wrote:
> > >> I agree with Noah's opinion that we should stick to the historical
> > >> behavior in the interval I/O functions.  There is not enough solidity
> > >> in the "this is grammatically wrong" argument to justify taking any
> > >> risk of application breakage, and it seems like there is some risk of
> > >> that there.
> > 
> > > Are you saying we should revert the patch and leave the plurals
> > > inconsistent in different places?  We were already expressing -1 as
> > > singular in most of the places I saw.

Which places did you find?  I used the following to check for code like you
were adding, but it probably missed things:

$ git grep '"s"' 5da9868^ | grep abs
5da9868^:src/backend/utils/adt/datetime.c:						(abs(sec) != 1 || fsec != 0) ? "s" : "");
5da9868^:src/interfaces/ecpg/pgtypeslib/interval.c:						(abs(sec) != 1 || fsec != 0) ? "s" : "");

> > As far as the changes in datetime.c and interval.c are concerned,
> > yes.  I don't care too much about what you did in fe-print.c,
> 
> Well, we should then add a comment that this is inconsistent but should
> not be changed.

No objection to having a comment.





^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: BUG #16939: Plural interval for negative singular
  2021-03-23 05:21 BUG #16939: Plural interval for negative singular PG Bug reporting form <noreply@postgresql.org>
  2021-03-23 16:03 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-25 11:57   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-26 16:31     ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 16:45       ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 16:54         ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 17:02           ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 17:06             ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-27 06:13               ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
@ 2021-04-27 20:10                 ` Bruce Momjian <bruce@momjian.us>
  2021-04-28 05:44                   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  0 siblings, 1 reply; 20+ messages in thread

From: Bruce Momjian @ 2021-04-27 20:10 UTC (permalink / raw)
  To: Noah Misch <noah@leadboat.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; neverov.max@gmail.com; pgsql-bugs@lists.postgresql.org

On Mon, Apr 26, 2021 at 11:13:29PM -0700, Noah Misch wrote:
> On Mon, Apr 26, 2021 at 01:06:16PM -0400, Bruce Momjian wrote:
> > On Mon, Apr 26, 2021 at 01:02:44PM -0400, Tom Lane wrote:
> > > Bruce Momjian <bruce@momjian.us> writes:
> > > > On Mon, Apr 26, 2021 at 12:45:34PM -0400, Tom Lane wrote:
> > > >> I agree with Noah's opinion that we should stick to the historical
> > > >> behavior in the interval I/O functions.  There is not enough solidity
> > > >> in the "this is grammatically wrong" argument to justify taking any
> > > >> risk of application breakage, and it seems like there is some risk of
> > > >> that there.
> > > 
> > > > Are you saying we should revert the patch and leave the plurals
> > > > inconsistent in different places?  We were already expressing -1 as
> > > > singular in most of the places I saw.
> 
> Which places did you find?  I used the following to check for code like you
> were adding, but it probably missed things:
> 
> $ git grep '"s"' 5da9868^ | grep abs
> 5da9868^:src/backend/utils/adt/datetime.c:						(abs(sec) != 1 || fsec != 0) ? "s" : "");
> 5da9868^:src/interfaces/ecpg/pgtypeslib/interval.c:						(abs(sec) != 1 || fsec != 0) ? "s" : "");

Yes, I found the same like you did.  Are you saying we should remove the
abs() calls here so we are consistent?

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: BUG #16939: Plural interval for negative singular
  2021-03-23 05:21 BUG #16939: Plural interval for negative singular PG Bug reporting form <noreply@postgresql.org>
  2021-03-23 16:03 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-25 11:57   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-26 16:31     ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 16:45       ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 16:54         ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 17:02           ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 17:06             ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-27 06:13               ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-27 20:10                 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
@ 2021-04-28 05:44                   ` Noah Misch <noah@leadboat.com>
  2021-04-30 01:00                     ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  0 siblings, 1 reply; 20+ messages in thread

From: Noah Misch @ 2021-04-28 05:44 UTC (permalink / raw)
  To: Bruce Momjian <bruce@momjian.us>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; neverov.max@gmail.com; pgsql-bugs@lists.postgresql.org

On Tue, Apr 27, 2021 at 04:10:19PM -0400, Bruce Momjian wrote:
> On Mon, Apr 26, 2021 at 11:13:29PM -0700, Noah Misch wrote:
> > On Mon, Apr 26, 2021 at 01:06:16PM -0400, Bruce Momjian wrote:
> > > On Mon, Apr 26, 2021 at 01:02:44PM -0400, Tom Lane wrote:
> > > > Bruce Momjian <bruce@momjian.us> writes:
> > > > > On Mon, Apr 26, 2021 at 12:45:34PM -0400, Tom Lane wrote:
> > > > >> I agree with Noah's opinion that we should stick to the historical
> > > > >> behavior in the interval I/O functions.  There is not enough solidity
> > > > >> in the "this is grammatically wrong" argument to justify taking any
> > > > >> risk of application breakage, and it seems like there is some risk of
> > > > >> that there.
> > > > 
> > > > > Are you saying we should revert the patch and leave the plurals
> > > > > inconsistent in different places?  We were already expressing -1 as
> > > > > singular in most of the places I saw.
> > 
> > Which places did you find?  I used the following to check for code like you
> > were adding, but it probably missed things:
> > 
> > $ git grep '"s"' 5da9868^ | grep abs
> > 5da9868^:src/backend/utils/adt/datetime.c:						(abs(sec) != 1 || fsec != 0) ? "s" : "");
> > 5da9868^:src/interfaces/ecpg/pgtypeslib/interval.c:						(abs(sec) != 1 || fsec != 0) ? "s" : "");
> 
> 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.





^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: BUG #16939: Plural interval for negative singular
  2021-03-23 05:21 BUG #16939: Plural interval for negative singular PG Bug reporting form <noreply@postgresql.org>
  2021-03-23 16:03 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-25 11:57   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-26 16:31     ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 16:45       ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 16:54         ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 17:02           ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 17:06             ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-27 06:13               ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-27 20:10                 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-28 05:44                   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
@ 2021-04-30 01:00                     ` Bruce Momjian <bruce@momjian.us>
  2021-04-30 03:15                       ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-05-01 14:55                       ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  0 siblings, 2 replies; 20+ messages in thread

From: Bruce Momjian @ 2021-04-30 01:00 UTC (permalink / raw)
  To: Noah Misch <noah@leadboat.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; neverov.max@gmail.com; pgsql-bugs@lists.postgresql.org

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) download 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("</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.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
^ permalink raw reply [nested|flat] 20+ messages in thread

* Re: BUG #16939: Plural interval for negative singular
  2021-03-23 05:21 BUG #16939: Plural interval for negative singular PG Bug reporting form <noreply@postgresql.org>
  2021-03-23 16:03 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-25 11:57   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-26 16:31     ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 16:45       ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 16:54         ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 17:02           ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 17:06             ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-27 06:13               ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-27 20:10                 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-28 05:44                   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-30 01:00                     ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
@ 2021-04-30 03:15                       ` Noah Misch <noah@leadboat.com>
  1 sibling, 0 replies; 20+ messages in thread

From: Noah Misch @ 2021-04-30 03:15 UTC (permalink / raw)
  To: Bruce Momjian <bruce@momjian.us>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; neverov.max@gmail.com; pgsql-bugs@lists.postgresql.org

On Thu, Apr 29, 2021 at 09:00:04PM -0400, Bruce Momjian wrote:
> 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)

Nice.  The legendary consistency of PostgreSQL remains intact.

> Attached is a patch which reverts 9ee7d533da and 5da9868ed9, and adds a
> comment about the "ago" usage.

Looks good.





^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: BUG #16939: Plural interval for negative singular
  2021-03-23 05:21 BUG #16939: Plural interval for negative singular PG Bug reporting form <noreply@postgresql.org>
  2021-03-23 16:03 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-25 11:57   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-26 16:31     ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 16:45       ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 16:54         ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 17:02           ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 17:06             ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-27 06:13               ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-27 20:10                 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-28 05:44                   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-30 01:00                     ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
@ 2021-05-01 14:55                       ` Bruce Momjian <bruce@momjian.us>
  1 sibling, 0 replies; 20+ messages in thread

From: Bruce Momjian @ 2021-05-01 14:55 UTC (permalink / raw)
  To: Noah Misch <noah@leadboat.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; neverov.max@gmail.com; pgsql-bugs@lists.postgresql.org

On Thu, Apr 29, 2021 at 09:00:04PM -0400, Bruce Momjian wrote:
> 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.  :-)

Here is a clearer example:

	SET intervalstyle = postgres_verbose;
	
	SELECT '2020-01-01 00:00:01'::timestamptz - '2020-01-01 00:00:00'::timestamptz;
	 ?column?
	----------
	 @ 1 sec
	
	SELECT '2020-01-01 00:00:00'::timestamptz - '2020-01-01 00:00:01'::timestamptz;
	  ?column?
	-------------
	 @ 1 sec ago
	
Patch applied.  Thanks for the research.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: BUG #16939: Plural interval for negative singular
  2021-03-23 05:21 BUG #16939: Plural interval for negative singular PG Bug reporting form <noreply@postgresql.org>
  2021-03-23 16:03 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-25 11:57   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-26 16:31     ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 16:45       ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 16:54         ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 17:02           ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-04-27 22:50             ` Alvaro Herrera <alvherre@alvh.no-ip.org>
  2021-04-27 23:15               ` Re: BUG #16939: Plural interval for negative singular Gavin Flower <GavinFlower@archidevsys.co.nz>
  1 sibling, 1 reply; 20+ messages in thread

From: Alvaro Herrera @ 2021-04-27 22:50 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Bruce Momjian <bruce@momjian.us>; Noah Misch <noah@leadboat.com>; neverov.max@gmail.com; pgsql-bugs@lists.postgresql.org

On 2021-Apr-26, Tom Lane wrote:

> (I shy gently away from the fact that that fe-print.c code is
> relentlessly untranslatable.)

It's also unused.  Maybe we should consider removing instead ...

-- 
Álvaro Herrera       Valdivia, Chile
"I can see support will not be a problem.  10 out of 10."    (Simon Wittber)
      (http://archives.postgresql.org/pgsql-general/2004-12/msg00159.php)





^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: BUG #16939: Plural interval for negative singular
  2021-03-23 05:21 BUG #16939: Plural interval for negative singular PG Bug reporting form <noreply@postgresql.org>
  2021-03-23 16:03 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-25 11:57   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-26 16:31     ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 16:45       ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 16:54         ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 17:02           ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-27 22:50             ` Re: BUG #16939: Plural interval for negative singular Alvaro Herrera <alvherre@alvh.no-ip.org>
@ 2021-04-27 23:15               ` Gavin Flower <GavinFlower@archidevsys.co.nz>
  2021-04-27 23:44                 ` Re: BUG #16939: Plural interval for negative singular Alvaro Herrera <alvherre@alvh.no-ip.org>
  0 siblings, 1 reply; 20+ messages in thread

From: Gavin Flower @ 2021-04-27 23:15 UTC (permalink / raw)
  To: Alvaro Herrera <alvherre@alvh.no-ip.org>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Bruce Momjian <bruce@momjian.us>; Noah Misch <noah@leadboat.com>; neverov.max@gmail.com; pgsql-bugs@lists.postgresql.org

On 28/04/21 10:50 am, Alvaro Herrera wrote:
> On 2021-Apr-26, Tom Lane wrote:
>
>> (I shy gently away from the fact that that fe-print.c code is
>> relentlessly untranslatable.)
> It's also unused.  Maybe we should consider removing instead ...
>
fe-print.c is in the source tree for LibreOffice 7.2, when I downloaded git master:
... git-libreoffice/workdir/UnpackedTarball/postgresql/src/interfaces/libpq/fe-print.c'






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: BUG #16939: Plural interval for negative singular
  2021-03-23 05:21 BUG #16939: Plural interval for negative singular PG Bug reporting form <noreply@postgresql.org>
  2021-03-23 16:03 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-25 11:57   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-26 16:31     ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 16:45       ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 16:54         ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 17:02           ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-27 22:50             ` Re: BUG #16939: Plural interval for negative singular Alvaro Herrera <alvherre@alvh.no-ip.org>
  2021-04-27 23:15               ` Re: BUG #16939: Plural interval for negative singular Gavin Flower <GavinFlower@archidevsys.co.nz>
@ 2021-04-27 23:44                 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
  2021-04-28 00:09                   ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 20+ messages in thread

From: Alvaro Herrera @ 2021-04-27 23:44 UTC (permalink / raw)
  To: Gavin Flower <GavinFlower@archidevsys.co.nz>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Bruce Momjian <bruce@momjian.us>; Noah Misch <noah@leadboat.com>; neverov.max@gmail.com; pgsql-bugs@lists.postgresql.org

On 2021-Apr-28, Gavin Flower wrote:

> On 28/04/21 10:50 am, Alvaro Herrera wrote:
> > On 2021-Apr-26, Tom Lane wrote:
> > 
> > > (I shy gently away from the fact that that fe-print.c code is
> > > relentlessly untranslatable.)
> > It's also unused.  Maybe we should consider removing instead ...
> > 
> fe-print.c is in the source tree for LibreOffice 7.2, when I downloaded git master:
> ... git-libreoffice/workdir/UnpackedTarball/postgresql/src/interfaces/libpq/fe-print.c'

I bet that's because they import the whole src/interfaces/libpq, but
don't actually use the functions provided by that file.

... updates local clone ...

Nope -- grepping for PQprint or PQdisplayTup doesn't yield anything in
the libreoffice git repo.

-- 
Álvaro Herrera                            39°49'30"S 73°17'W
"Once again, thank you and all of the developers for your hard work on
PostgreSQL.  This is by far the most pleasant management experience of
any database I've worked on."                             (Dan Harris)
http://archives.postgresql.org/pgsql-performance/2006-04/msg00247.php





^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: BUG #16939: Plural interval for negative singular
  2021-03-23 05:21 BUG #16939: Plural interval for negative singular PG Bug reporting form <noreply@postgresql.org>
  2021-03-23 16:03 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-25 11:57   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-26 16:31     ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 16:45       ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 16:54         ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 17:02           ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-27 22:50             ` Re: BUG #16939: Plural interval for negative singular Alvaro Herrera <alvherre@alvh.no-ip.org>
  2021-04-27 23:15               ` Re: BUG #16939: Plural interval for negative singular Gavin Flower <GavinFlower@archidevsys.co.nz>
  2021-04-27 23:44                 ` Re: BUG #16939: Plural interval for negative singular Alvaro Herrera <alvherre@alvh.no-ip.org>
@ 2021-04-28 00:09                   ` Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-28 03:43                     ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 20+ messages in thread

From: Tom Lane @ 2021-04-28 00:09 UTC (permalink / raw)
  To: Alvaro Herrera <alvherre@alvh.no-ip.org>; +Cc: Gavin Flower <GavinFlower@archidevsys.co.nz>; Bruce Momjian <bruce@momjian.us>; Noah Misch <noah@leadboat.com>; neverov.max@gmail.com; pgsql-bugs@lists.postgresql.org

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Nope -- grepping for PQprint or PQdisplayTup doesn't yield anything in
> the libreoffice git repo.

Hm.  We actually document PQprint.  While the other two entry points
in that file aren't documented, they are listed in exports.txt,
which means somebody *could* be calling them.  I think we'd at least
have to go through a long deprecation period before we could consider
removing them.

There are also questions of whether we can remove entry points
without drawing the wrath of ABI checkers.  I suppose we could
dodge that by reducing them to code that returns an error.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: BUG #16939: Plural interval for negative singular
  2021-03-23 05:21 BUG #16939: Plural interval for negative singular PG Bug reporting form <noreply@postgresql.org>
  2021-03-23 16:03 ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-25 11:57   ` Re: BUG #16939: Plural interval for negative singular Noah Misch <noah@leadboat.com>
  2021-04-26 16:31     ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 16:45       ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-26 16:54         ` Re: BUG #16939: Plural interval for negative singular Bruce Momjian <bruce@momjian.us>
  2021-04-26 17:02           ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-27 22:50             ` Re: BUG #16939: Plural interval for negative singular Alvaro Herrera <alvherre@alvh.no-ip.org>
  2021-04-27 23:15               ` Re: BUG #16939: Plural interval for negative singular Gavin Flower <GavinFlower@archidevsys.co.nz>
  2021-04-27 23:44                 ` Re: BUG #16939: Plural interval for negative singular Alvaro Herrera <alvherre@alvh.no-ip.org>
  2021-04-28 00:09                   ` Re: BUG #16939: Plural interval for negative singular Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-04-28 03:43                     ` Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 20+ messages in thread

From: Tom Lane @ 2021-04-28 03:43 UTC (permalink / raw)
  To: Bruce Momjian <bruce@momjian.us>; +Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>; Gavin Flower <GavinFlower@archidevsys.co.nz>; Noah Misch <noah@leadboat.com>; neverov.max@gmail.com; pgsql-bugs@lists.postgresql.org

I wrote:
> Hm.  We actually document PQprint.  While the other two entry points
> in that file aren't documented, they are listed in exports.txt,
> which means somebody *could* be calling them.  I think we'd at least
> have to go through a long deprecation period before we could consider
> removing them.

Meanwhile, returning to the point at hand in this thread: AFAICS
from a quick review of the git logs, the last intentional changes
of user-visible behavior in fe-print.c were more than 20 years ago.
Since then it's only seen in-passing bug fixes, such as libpq-wide
fixes of signal handling.

So, to the extent that this code has any use at all it's for
legacy applications.  I don't think we should be making random
changes in its behavior for at-best-weak reasons.

In short, I'm now a vote to revert every single bit of 5da9868ed.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 20+ messages in thread


end of thread, other threads:[~2021-05-01 14:55 UTC | newest]

Thread overview: 20+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-03-23 05:21 BUG #16939: Plural interval for negative singular PG Bug reporting form <noreply@postgresql.org>
2021-03-23 16:03 ` Bruce Momjian <bruce@momjian.us>
2021-03-30 22:35   ` Bruce Momjian <bruce@momjian.us>
2021-04-25 11:57   ` Noah Misch <noah@leadboat.com>
2021-04-26 16:31     ` Bruce Momjian <bruce@momjian.us>
2021-04-26 16:45       ` Tom Lane <tgl@sss.pgh.pa.us>
2021-04-26 16:54         ` Bruce Momjian <bruce@momjian.us>
2021-04-26 17:02           ` Tom Lane <tgl@sss.pgh.pa.us>
2021-04-26 17:06             ` Bruce Momjian <bruce@momjian.us>
2021-04-27 06:13               ` Noah Misch <noah@leadboat.com>
2021-04-27 20:10                 ` Bruce Momjian <bruce@momjian.us>
2021-04-28 05:44                   ` Noah Misch <noah@leadboat.com>
2021-04-30 01:00                     ` Bruce Momjian <bruce@momjian.us>
2021-04-30 03:15                       ` Noah Misch <noah@leadboat.com>
2021-05-01 14:55                       ` Bruce Momjian <bruce@momjian.us>
2021-04-27 22:50             ` Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-04-27 23:15               ` Gavin Flower <GavinFlower@archidevsys.co.nz>
2021-04-27 23:44                 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-04-28 00:09                   ` Tom Lane <tgl@sss.pgh.pa.us>
2021-04-28 03:43                     ` Tom Lane <tgl@sss.pgh.pa.us>

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