Re: Parallel pg_dump's error reporting doesn't work worth squat

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: tgl(at)sss(dot)pgh(dot)pa(dot)us
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Parallel pg_dump's error reporting doesn't work worth squat
Date: 2016-05-27 08:38:05
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

At Thu, 26 May 2016 12:15:29 -0400, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote in <8273(dot)1464279329(at)sss(dot)pgh(dot)pa(dot)us>
> Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> writes:
> > At Wed, 25 May 2016 10:11:28 -0400, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote in <24577(dot)1464185488(at)sss(dot)pgh(dot)pa(dot)us>
> >> The only case
> >> that is certain to work is switches before non-switch arguments, and so
> >> we should not give any documentation examples in the other order. On a
> >> platform using GNU getopt(), getopt() will rearrange the argv[] array to
> >> make such cases work, but non-GNU getopt() doesn't do that (and I don't
> >> really trust the GNU version not to get confused, either).
> > Yeah, I knew it after reading port/getopt_long.c. But the error
> > message seems saying nothing about that (at least to me). And
> > those accumstomed to the GNU getopt's behavior will fail to get
> > the point of the message. The documentation also doesn't
> > explicitly saying about the restriction; it is only implicitly
> > shown in synopsis. How about something like the following
> > message? (The patch attached looks too dependent on the specific
> > behavior of our getopt_long.c, but doing it in getopt_long.c also
> > seems to be wrong..)
> It's not a bad idea to try to improve our response to this situation,
> but I think this patch needs more work. I wonder in particular why
> you're not basing the variant error message on the first unprocessed
> argument starting with '-', rather than looking at the word before it.

Sorry, it just comes from my carelessness. It is correct to do
what you wrote as above. And it is also the cause of my obscure
error message.

> Another thought is that the message is fairly unhelpful because it does
> not show where in the arguments things went wrong. Maybe something
> more like
> if (argv[optind][0] == '-')
> fprintf(stderr, _("%s: misplaced option \"%s\": options must appear before non-option arguments\n"),
> progname, argv[optind]);
> else
> // existing message
> In any case, if we wanted to do something about this scenario, we should
> do it consistently across all our programs, not just pg_dump. I count
> 25 appearances of that "too many command-line arguments" message...

Although I suppose no one has ever complained before about that,
and the same thing will occur on the other new programs even if
the programs are fixed now..

I'll consider more on it for some time..

Thank you for the suggestion.


Kyotaro Horiguchi
NTT Open Source Software Center

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro HORIGUCHI 2016-05-27 08:38:26 Re: Parallel pg_dump's error reporting doesn't work worth squat
Previous Message Andrew Gierth 2016-05-27 07:17:17 Re: Allow COPY to use parameters