Re: refactoring - standardize integer parsing in front-end utilities

From: Joe Nelson <joe(at)begriffs(dot)com>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
Cc: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, david(dot)rowley(at)2ndquadrant(dot)com, ashu(dot)coek88(at)gmail(dot)com, robertmhaas(at)gmail(dot)com, michael(at)paquier(dot)xyz, tomas(dot)vondra(at)2ndquadrant(dot)com, surafel3000(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org, alvherre(at)2ndquadrant(dot)com
Subject: Re: refactoring - standardize integer parsing in front-end utilities
Date: 2020-01-12 21:43:38
Message-ID: 20200112214338.dnmeacld5bwyr3fu@begriffs.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Joe Nelson wrote:
> > I see this patch has been moved to the next commitfest. What's the next
> > step, does it need another review?

Peter Eisentraut wrote:
> I think you need to promote your patch better.

Thanks for taking the time to revive this thread.

Quick sales pitch for the patch:

* Standardizes bounds checking and error message format in utilities
pg_standby, pg_basebackup, pg_receivewal, pg_recvlogical, pg_ctl,
pg_dump, pg_restore, pg_upgrade, pgbench, reindexdb, and vacuumdb
* Removes Undefined Behavior caused by atoi() as described
in the C99 standard, section 7.20.1
* Unifies integer parsing between the front- and back-end using
functions provided in https://commitfest.postgresql.org/25/2272/

In reality I doubt my patch is fixing any pressing problem, it's just a
small refactor.

> The thread subject and the commit fest entry title are somewhat
> nonsensical and no longer match what the patch actually does.

I thought changing the subject line might be discouraged, but since you
suggest doing it, I just did. Updated the title of the commitfest entry
https://commitfest.postgresql.org/26/2197/ as well.

> The patch contains no commit message

Does this list not accept plain patches, compatible with git-apply?
(Maybe your point is that I should make it as easy for committers as
possible, and asking them to invent a commit message is just extra
work.)

> and no documentation or test changes

The interfaces of the utilities remain the same. Same flags. The only
change would be the error messages produced for incorrect values.

The tests I ran passed successfully, but perhaps there were others I
didn't try running and should have.

> Moreover, a lot of this error message tweaking is opinion-based, so
> it's more burdensome to do an objective review. This patch is
> competing for attention against more than 200 other patches that have
> more going for them in these aspects.

True. I think the code looks nicer and the errors are more informative,
but I'll leave it in the community's hands to determine if this is
something they want.

Once again, I appreciate your taking the time to help me with this
process.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2020-01-12 23:03:37 Question regarding heap_multi_insert documentation
Previous Message Noah Misch 2020-01-12 20:59:59 Re: Consolidate 'unique array values' logic into a reusable function?