Обсуждение: pg_dump, pg_basebackup don't error out with wrong option for "--format"

Hi,

I noticed that the pg_dump and pg_basebackup are not erroring out when
"--fo"/"--for"/"--form"/"--forma"/" are specified(use cases 1 - 4, 7 -
9) whereas it fails if a pattern that doesn't match with "format" is
specified (use case 5, 10). This seems to be true only for "--format"
option, for other options it just errors out (use case 6). Why is the
behaviour for "--format" isn't consistent?

Is it something expected? Is the option parsing logic here buggy?

Thoughts?

Use cases:
1) ./pg_dump --dbname=postgres --host=localhost --port=5432
--username=bharath --no-password --fo=plain --file=mydump.sql
2) ./pg_dump --dbname=postgres --host=localhost --port=5432
--username=bharath --no-password --for=plain --file=mydump.sql
3) ./pg_dump --dbname=postgres --host=localhost --port=5432
--username=bharath --no-password --form=plain --file=mydump.sql
4) ./pg_dump --dbname=postgres --host=localhost --port=5432
--username=bharath --no-password --forma=plain --file=mydump.sql
5) ./pg_dump --dbname=postgres --host=localhost --port=5432
--username=bharath --no-password --foo=plain --file=mydump.sql
6) ./pg_dump --dbname=postgres --host=localhost --port=5432
--username=bharath --no-password --format=plain --fi=mydump.sql
7) ./pg_basebackup -D bkupdata --fo=plain
8) ./pg_basebackup -D bkupdata --for=plain
9) ./pg_basebackup -D bkupdata --forma=plain
10) ./pg_basebackup -D bkupdata --foo=plain

Regards,
Bharath Rupireddy.

On Thu, Nov 25, 2021 at 10:44 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> Hi,
>
> I noticed that the pg_dump and pg_basebackup are not erroring out when
> "--fo"/"--for"/"--form"/"--forma"/" are specified(use cases 1 - 4, 7 -
> 9) whereas it fails if a pattern that doesn't match with "format" is
> specified (use case 5, 10). This seems to be true only for "--format"
> option, for other options it just errors out (use case 6). Why is the
> behaviour for "--format" isn't consistent?
>
> Is it something expected? Is the option parsing logic here buggy?

I think for parsing we use getopt_long(), as per that if you use the
prefix of the string and that is not conflicting with any other option
then that is allowed.  So --fo, --for all are accepted, --f will not
be accepted because --file and --format will conflict, --foo is also
not allowed because it is not a valid prefix string of any valid
option string.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

On Thu, Nov 25, 2021 at 11:02 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Thu, Nov 25, 2021 at 10:44 AM Bharath Rupireddy
> <bharath.rupireddyforpostgres@gmail.com> wrote:
> >
> > Hi,
> >
> > I noticed that the pg_dump and pg_basebackup are not erroring out when
> > "--fo"/"--for"/"--form"/"--forma"/" are specified(use cases 1 - 4, 7 -
> > 9) whereas it fails if a pattern that doesn't match with "format" is
> > specified (use case 5, 10). This seems to be true only for "--format"
> > option, for other options it just errors out (use case 6). Why is the
> > behaviour for "--format" isn't consistent?
> >
> > Is it something expected? Is the option parsing logic here buggy?
>
> I think for parsing we use getopt_long(), as per that if you use the
> prefix of the string and that is not conflicting with any other option
> then that is allowed.  So --fo, --for all are accepted, --f will not
> be accepted because --file and --format will conflict, --foo is also
> not allowed because it is not a valid prefix string of any valid
> option string.

Yeah, that makes sense. Thanks.

Regards,
Bharath Rupireddy.

On Thu, Nov 25, 2021 at 11:22:11AM +0530, Bharath Rupireddy wrote:
> On Thu, Nov 25, 2021 at 11:02 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>> I think for parsing we use getopt_long(), as per that if you use the
>> prefix of the string and that is not conflicting with any other option
>> then that is allowed.  So --fo, --for all are accepted, --f will not
>> be accepted because --file and --format will conflict, --foo is also
>> not allowed because it is not a valid prefix string of any valid
>> option string.
>
> Yeah, that makes sense. Thanks.

It is worth noting that getopt_long() is a GNU extension and not
directly something defined in POSIX, but it is so popular that it
expanded everywhere.  This option anme handling is quite common across
everything that uses getopt_long(), actually, and erroring on
non-exact option names would break a bunch of existing use cases as it
is possible to save some characters if getopt_long() is sure of the
uniqueness of the option found.
--
Michael