Re: Parallel pg_dump's error reporting doesn't work worth squat - Mailing list pgsql-hackers

At Wed, 25 May 2016 10:11:28 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in <24577.1464185488@sss.pgh.pa.us>
> Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:
> > I tried it on Windows 7/64 but first of all, I'm surprised to see
> > that the following command line gets an error but it would be
> > fine because it is consistent with what is written in the manual.
> 
> > | >pg_dump postgres://horiguti:hoge@localhost/postgres --jobs=9 -Fd -f testdump
> > | pg_dump: too many command-line arguments (first is "--jobs=9")
> > | Try "pg_dump --help" for more information.
> 
> Where do you see an example like that?  We should fix it.

Sorry for the confusing sentence. The command line came from your
first mail starting this thread:p And "consistent with manual"
means that the command line results in an error (even only on
Windows) since it is difrerent from the document. No such example
has been seen in the documentation AFAICS.

https://www.postgresql.org/message-id/2458.1450894615@sss.pgh.pa.us
https://www.postgresql.org/docs/9.6/static/app-pgdump.html

>  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..)

| >pg_dump hoge -f
| pg_dump: non-option arguments should not precede options.


> > I might be wrong with something, but pg_dump seems to have some
> > issues in thread handling.
> 
> Wouldn't surprise me ... there's a lot of code in there depending on
> static variables, and we've only tried to thread-proof a few.  Still,
> I think that's a separate issue from what this patch is addressing.

Sounds reasonable. I look into this further.
I see no other functional problem in this patch.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 1267afb..52e9094 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -546,9 +546,13 @@ main(int argc, char **argv)    /* Complain if any arguments remain */    if (optind < argc)    {
-        fprintf(stderr, _("%s: too many command-line arguments (first is \"%s\")\n"),
-                progname, argv[optind]);
-        fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+        if (optind > 0 && argv[optind - 1][0] != '-')
+            fprintf(stderr, _("%s: non-option arguments should not precede options.\n"),
+                    progname);
+        else
+            fprintf(stderr, _("%s: too many command-line arguments (first is \"%s\")\n"),
+                    progname, argv[optind]);
+        fprintf(stderr, _("Non-Try \"%s --help\" for more information.\n"),                progname);
exit_nicely(1);   }