Обсуждение: [HACKERS] invalid combination of options "-D - -F t -X stream" in pg_basebackup

Hi,

Isn't it better to forbid the conbination of the options "-D -", "-F t" and
"-X stream" in pg_basebackup? This is obviously invalid setting and the docs
warns this as follows. But currently users can specify such setting and
pg_basebackup can exit unexpectedly with an error.

-----------------------
If the value - (dash) is specified as target directory, the tar contents will
be written to standard output, suitable for piping to for example gzip.
This is only possible if the cluster has no additional tablespaces.
-----------------------

Regards,

-- 
Fujii Masao

On Tue, Dec 20, 2016 at 1:43 AM, Magnus Hagander <magnus@hagander.net> wrote:
>
>
> On Mon, Dec 19, 2016 at 5:39 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
>>
>> Hi,
>>
>> Isn't it better to forbid the conbination of the options "-D -", "-F t"
>> and
>> "-X stream" in pg_basebackup? This is obviously invalid setting and the
>> docs
>> warns this as follows. But currently users can specify such setting and
>> pg_basebackup can exit unexpectedly with an error.
>>
>> -----------------------
>> If the value - (dash) is specified as target directory, the tar contents
>> will
>> be written to standard output, suitable for piping to for example gzip.
>> This is only possible if the cluster has no additional tablespaces.
>> -----------------------
>
>
> Yes, definitely. I'd say that's an oversight in implementing the support for
> stream-to-tar that it did not detect this issue.
>
> Do you want to provide a patch for it, or should I?

What about the attached patch?

+        fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+                progname);

I added the above hint message because other codes checking invalid
options also have such hint messages. But there is no additional
useful information about valid combination of options in the help
messages, so I'm a bit tempted to remove the above hint message.

Regards,

-- 
Fujii Masao

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Dec 20, 2016 at 9:22 PM, Magnus Hagander <magnus@hagander.net> wrote:
>
>
> On Tue, Dec 20, 2016 at 6:56 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
>>
>> On Tue, Dec 20, 2016 at 1:43 AM, Magnus Hagander <magnus@hagander.net>
>> wrote:
>> >
>> >
>> > On Mon, Dec 19, 2016 at 5:39 PM, Fujii Masao <masao.fujii@gmail.com>
>> > wrote:
>> >>
>> >> Hi,
>> >>
>> >> Isn't it better to forbid the conbination of the options "-D -", "-F t"
>> >> and
>> >> "-X stream" in pg_basebackup? This is obviously invalid setting and the
>> >> docs
>> >> warns this as follows. But currently users can specify such setting and
>> >> pg_basebackup can exit unexpectedly with an error.
>> >>
>> >> -----------------------
>> >> If the value - (dash) is specified as target directory, the tar
>> >> contents
>> >> will
>> >> be written to standard output, suitable for piping to for example gzip.
>> >> This is only possible if the cluster has no additional tablespaces.
>> >> -----------------------
>> >
>> >
>> > Yes, definitely. I'd say that's an oversight in implementing the support
>> > for
>> > stream-to-tar that it did not detect this issue.
>> >
>> > Do you want to provide a patch for it, or should I?
>>
>> What about the attached patch?
>>
>> +        fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
>> +                progname);
>>
>> I added the above hint message because other codes checking invalid
>> options also have such hint messages. But there is no additional
>> useful information about valid combination of options in the help
>> messages, so I'm a bit tempted to remove the above hint message.
>
>
> Looks good to me.

Thanks for the review! Pushed.

I left the hint message for consistency with other code.

Regards,

-- 
Fujii Masao