Re: invalid combination of options "-D - -F t -X stream" in pg_basebackup

From: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
To: Magnus Hagander <magnus(at)hagander(dot)net>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: invalid combination of options "-D - -F t -X stream" in pg_basebackup
Date: 2016-12-21 11:29:06
Message-ID: CAHGQGwEi-m74E2iKqhuAcsAcxHM21Nk52bmq-ch1OjsUX6aWkg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Dec 20, 2016 at 9:22 PM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
>
>
> On Tue, Dec 20, 2016 at 6:56 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>
>> On Tue, Dec 20, 2016 at 1:43 AM, Magnus Hagander <magnus(at)hagander(dot)net>
>> wrote:
>> >
>> >
>> > On Mon, Dec 19, 2016 at 5:39 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Bapat 2016-12-21 11:36:38 multi-level partitions and partition-wise joins
Previous Message Joel Jacobson 2016-12-21 11:01:57 Re: SET NOT NULL [NOT VALID / CONCURRENTLY]?