Horton

Thread

Commits

  1. Error out on too many command-line arguments

  1. pg_upgrade: Error out on too many command-line arguments

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-08-25T08:51:47Z

    I propose the attached patch to make pg_upgrade error out on too many
command-line arguments.  This makes it match the behavior of other
PostgreSQL programs.

See [0] for an issue related to the lack of this check:

[0]:
https://www.postgresql.org/message-id/871sdbzizp.fsf%40jsievers.enova.com

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  2. Re: pg_upgrade: Error out on too many command-line arguments

    Julien Rouhaud <rjuju123@gmail.com> — 2019-08-25T09:01:56Z

    On Sun, Aug 25, 2019 at 10:52 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
>
> I propose the attached patch to make pg_upgrade error out on too many
> command-line arguments.  This makes it match the behavior of other
> PostgreSQL programs.
>
> See [0] for an issue related to the lack of this check:

+1

  3. Re: pg_upgrade: Error out on too many command-line arguments

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-08-25T14:29:59Z

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> I propose the attached patch to make pg_upgrade error out on too many
> command-line arguments.  This makes it match the behavior of other
> PostgreSQL programs.

+1 ... are we missing this anywhere else?

			regards, tom lane

  4. Re: pg_upgrade: Error out on too many command-line arguments

    Julien Rouhaud <rjuju123@gmail.com> — 2019-08-25T15:10:47Z

    On Sun, Aug 25, 2019 at 4:30 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> > I propose the attached patch to make pg_upgrade error out on too many
> > command-line arguments.  This makes it match the behavior of other
> > PostgreSQL programs.
>
> +1 ... are we missing this anywhere else?

I did some searching, and oid2name.c is also missing this.

  5. Re: pg_upgrade: Error out on too many command-line arguments

    Ibrar Ahmed <ibrar.ahmad@gmail.com> — 2019-08-25T18:50:44Z

    On Sun, Aug 25, 2019 at 8:39 PM Julien Rouhaud <rjuju123@gmail.com> wrote:

> On Sun, Aug 25, 2019 at 4:30 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >
> > Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> > > I propose the attached patch to make pg_upgrade error out on too many
> > > command-line arguments.  This makes it match the behavior of other
> > > PostgreSQL programs.
> >
> > +1 ... are we missing this anywhere else?
>
> I did some searching, and oid2name.c is also missing this.
>
> Yes, "oid2name" missing that check too.



-- 
Ibrar Ahmed

  6. Re: pg_upgrade: Error out on too many command-line arguments

    Michael Paquier <michael@paquier.xyz> — 2019-08-26T04:45:13Z

    On Sun, Aug 25, 2019 at 05:10:47PM +0200, Julien Rouhaud wrote:
> I did some searching, and oid2name.c is also missing this.

And pgbench, no?
--
Michael

  7. Re: pg_upgrade: Error out on too many command-line arguments

    Ibrar Ahmed <ibrar.ahmad@gmail.com> — 2019-08-26T15:45:02Z

    On Mon, Aug 26, 2019 at 9:46 AM Michael Paquier <michael@paquier.xyz> wrote:

> On Sun, Aug 25, 2019 at 05:10:47PM +0200, Julien Rouhaud wrote:
> > I did some searching, and oid2name.c is also missing this.
>
> And pgbench, no?
>

Yes, the patch is slightly different.


> --
> Michael
>


-- 
Ibrar Ahmed

  8. Re: pg_upgrade: Error out on too many command-line arguments

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-08-29T18:52:21Z

    On 2019-08-26 17:45, Ibrar Ahmed wrote:
> On Mon, Aug 26, 2019 at 9:46 AM Michael Paquier <michael@paquier.xyz
> <mailto:michael@paquier.xyz>> wrote:
> 
>     On Sun, Aug 25, 2019 at 05:10:47PM +0200, Julien Rouhaud wrote:
>     > I did some searching, and oid2name.c is also missing this.
> 
>     And pgbench, no?
> 
>  
> Yes, the patch is slightly different.

Thanks, pushed all that together.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  9. Re: pg_upgrade: Error out on too many command-line arguments

    Fabien COELHO <coelho@cri.ensmp.fr> — 2019-08-30T06:44:28Z

    Hello Peter,

>>     On Sun, Aug 25, 2019 at 05:10:47PM +0200, Julien Rouhaud wrote:
>>    > I did some searching, and oid2name.c is also missing this.
>>
>>     And pgbench, no?
>>  
>> Yes, the patch is slightly different.
>
> Thanks, pushed all that together.

Great!

Could we maintain coverage by adding a TAP test? See 1 liner attached.

-- 
Fabien.

  10. Re: pg_upgrade: Error out on too many command-line arguments

    Michael Paquier <michael@paquier.xyz> — 2019-08-30T06:59:05Z

    On Fri, Aug 30, 2019 at 08:44:28AM +0200, Fabien COELHO wrote:
> Could we maintain coverage by adding a TAP test? See 1 liner attached.

I don't see why not.  Perhaps this could be done for pgbench and
oid2name as well?
--
Michael

  11. Re: pg_upgrade: Error out on too many command-line arguments

    Fabien COELHO <coelho@cri.ensmp.fr> — 2019-08-30T07:14:07Z

    Bonjour Michaël,

> I don't see why not.  Perhaps this could be done for pgbench and
> oid2name as well?

This is for pgbench.

I did not found a TAP test in pg_upgrade, I do not think that it is worth 
creating one for that purpose. The "test.sh" script does not seem 
appropriate for this kind of coverage error test.

The TAP test for oid2name only includes basic checks, options and 
arguments are not even tested, and there is no infra for actual testing, 
eg starting a server… Improving that would be a much more significant 
effort that the one line I added to pgbench existing TAP test.

-- 
Fabien.

  12. Re: pg_upgrade: Error out on too many command-line arguments

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-08-30T13:54:13Z

    Fabien COELHO <coelho@cri.ensmp.fr> writes:
> Could we maintain coverage by adding a TAP test? See 1 liner attached.

Is this issue *really* worth expending test cycles on forevermore?

Test cycles are not free, and I see zero reason to think that a
check of this sort would ever catch any bugs.  Now, if you had a
way to detect that somebody had forgotten the case in some new
program, that would be interesting.

			regards, tom lane

  13. Re: pg_upgrade: Error out on too many command-line arguments

    Fabien COELHO <coelho@cri.ensmp.fr> — 2019-08-30T14:40:11Z

    Hello Tom,

>> Could we maintain coverage by adding a TAP test? See 1 liner attached.
>
> Is this issue *really* worth expending test cycles on forevermore?

With this argument consistently applied, postgres code coverage is 
consistently weak, with 25% of the code never executed, and 15% of 
functions never called. "psql" is abysmal, "libpq" is really weak.

> Test cycles are not free, and I see zero reason to think that a
> check of this sort would ever catch any bugs.  Now, if you had a
> way to detect that somebody had forgotten the case in some new
> program, that would be interesting.

It could get broken somehow, and the test would catch it?

That would be the only command which tests this feature?

This is a TAP test, not a test run on basic "make check". The cost is not 
measurable: pgbench 533 TAP tests run in 5 wallclock seconds, and this 
added test does not change that much.

Now, if you say you are against it, then it is rejected…

-- 
Fabien.

  14. Re: pg_upgrade: Error out on too many command-line arguments

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-08-30T14:47:48Z

    Fabien COELHO <coelho@cri.ensmp.fr> writes:
>> Is this issue *really* worth expending test cycles on forevermore?

> With this argument consistently applied, postgres code coverage is 
> consistently weak, with 25% of the code never executed, and 15% of 
> functions never called. "psql" is abysmal, "libpq" is really weak.

It's all a question of balance.  If you go too far in the other
direction, you end up with test suites that take an hour and a half
to run so nobody ever runs them (case in point, mysql).  I'm all for
improving coverage in meaningful ways --- but cases like this seem
unlikely to be worth ongoing expenditures of testing effort.

			regards, tom lane

  15. Re: pg_upgrade: Error out on too many command-line arguments

    Fabien COELHO <coelho@cri.ensmp.fr> — 2019-08-30T14:55:05Z

    >>> Is this issue *really* worth expending test cycles on forevermore?
>
>> With this argument consistently applied, postgres code coverage is
>> consistently weak, with 25% of the code never executed, and 15% of
>> functions never called. "psql" is abysmal, "libpq" is really weak.
>
> It's all a question of balance.  If you go too far in the other
> direction, you end up with test suites that take an hour and a half
> to run so nobody ever runs them (case in point, mysql).  I'm all for
> improving coverage in meaningful ways --- but cases like this seem
> unlikely to be worth ongoing expenditures of testing effort.

Sure.

I think there is room for several classes of tests, important ones always 
run and others run say by the farm, and I thought that is what TAP tests 
were for, given they are quite expensive anyway (eg most TAP test create 
their own postgres instance).

So for me the suggestion was appropriate for a pgbench-specific TAP test.

-- 
Fabien.