Re: pg_basebackup and error messages dependent on the order of the arguments

"Daniel Westermann (DWE)" <daniel(dot)westermann(at)dbi-services(dot)com> writes:
> shouldn't this give the same error message?

> $ pg_basebackup --checkpoint=fast --format=t --compress --pgdata=/var/tmp/dummy
> pg_basebackup: error: must specify output directory or backup target
> pg_basebackup: hint: Try "pg_basebackup --help" for more information.

> $ pg_basebackup --pgdata=/var/tmp/dummy --checkpoint=fast --format=t --compress
> pg_basebackup: option '--compress' requires an argument
> pg_basebackup: hint: Try "pg_basebackup --help" for more information.

> I don't see why the first case gives not the same message as the second, especially as the "output directory" is specified.

It appears that the first case treats "--pgdata=/var/tmp/dummy"
as the argument of --compress, and it doesn't bother to check that
that specifies a valid compression algorithm until much later.

As this example shows, we really ought to validate the compression
argument on sight in order to get sensible error messages. The
trouble is that for server-side compression the code wants to just
pass the string through to the server and not form its own opinion
as to whether it's a known algorithm.

Perhaps it would help if we simply rejected strings beginning
with a dash? I haven't tested, but roughly along the lines of

case 'Z':
+ /* we don't want to check the algorithm yet, but ... */
+ if (optarg[0] == '-')
+ pg_fatal("invalid compress option \"%s\", optarg);
backup_parse_compress_options(optarg, &compression_algorithm,
&compression_detail, &compressloc);
break;

regards, tom lane