Re: pg_restore --format= option(without any value) should report an error as pg_dump is reporting an error

Thanks Nathan for the review and feedback.

On Tue, 17 Mar 2026 at 01:04, Nathan Bossart <nathandbossart(at)gmail(dot)com>
wrote:
>
> On Sun, Mar 15, 2026 at 10:32:11PM +0530, Mahendra Singh Thalor wrote:
> > Here, I am attaching an updated patch for the review. I removed the
> > length check for host, port and format in pg_restore as we don't have
> > check in pg_dump also.
>
> Looks generally reasonable to me.
>
> > I think we don't need any test cases for host and port.
>
> Why not?

I did some more testing and found that if there is an empty string with
--port/--host, then we don't report any error because we don't validate
empty strings. This looks weird to me but this is happening with all tools
so I was not adding any test case for --host and --port.

Please see the test cases.
*test case1: ./psql -d postgres --port*
./psql: option '--port' requires an argument
psql: hint: Try "psql --help" for more information.

*test case 2: ./psql -d postgres --port=*
psql (19devel)
Type "help" for help.
postgres=# \q

*test case 3: ./psql -d postgres --port= 9*
psql: error: connection to server on socket "/tmp/.s.PGSQL.5432" failed:
FATAL: role "9" does not exist

In case 2 and 3, empty string is considered as valid value for --port and
the same is happening with --host and other options also. I think we should
add checks for all the tools to validate empty strings (tools should report
errors as value is required with these arguments.)

Please add your opinion.

>
> > If we want to backpatch, then I can make patches for back branches but
> > as of now, I am uploading a patch for master only.
>
> -1 for back-patching. --format seems to have been broken since pg_restore
> was first committed in 2000 (commit 500b62b057), so I don't sense any
> urgency here. Not to mention that someone might be relying on the current
> behavior.

Okay. I agree with you.

> > +command_fails_like(
> > + [ 'pg_restore', '-f -', '-F', 'p' ],
> > + qr/\Qpg_restore: error: archive format "p" is not supported;
please use psql\E/,
> > + 'pg_restore: unrecognized archive format p|plain');
>
> How does this test relate to this change?
>
> --
> nathan

Fixed. I removed this test case from the current patch.

Here, I am attaching an updated patch.

--
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com