Re: pg_basebackup and error messages dependent on the order of the arguments - Mailing list pgsql-hackers

From Tom Lane
Subject Re: pg_basebackup and error messages dependent on the order of the arguments
Date
Msg-id 1479283.1727714380@sss.pgh.pa.us
Whole thread Raw
In response to pg_basebackup and error messages dependent on the order of the arguments  ("Daniel Westermann (DWE)" <daniel.westermann@dbi-services.com>)
Responses Re: pg_basebackup and error messages dependent on the order of the arguments
List pgsql-hackers
"Daniel Westermann (DWE)" <daniel.westermann@dbi-services.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



pgsql-hackers by date:

Previous
From: Fujii Masao
Date:
Subject: Re: pg_basebackup and error messages dependent on the order of the arguments
Next
From: Antonin Houska
Date:
Subject: Re: [PoC] Federated Authn/z with OAUTHBEARER