public inbox for [email protected]  
help / color / mirror / Atom feed
From: Mahendra Singh Thalor <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Subject: Re: pg_restore --format= option(without any value) should report an error as pg_dump is reporting an error
Date: Sun, 15 Mar 2026 09:48:06 +0530
Message-ID: <CAKYtNApxyeeQbjxjFCgHwuv0T2TJvFaU6QSQaMU_yr1nP4Y8Fg@mail.gmail.com> (raw)
In-Reply-To: <CAKYtNApkh=Vy2DpNRCnEJmPpxNuksbAh_QBav=2fLmVjBhGwFw@mail.gmail.com>
References: <CAKYtNApkh=Vy2DpNRCnEJmPpxNuksbAh_QBav=2fLmVjBhGwFw@mail.gmail.com>

On Sun, 13 Apr 2025 at 18:32, Mahendra Singh Thalor <[email protected]> wrote:
>
> Hi,
> With "pg_restore --format=", we are not giving any error because in code, we are checking length of arg but pg_dump is reporting an error for the same option.
>
> For the consistency purpose, pg_dump and pg_restore both should report an error for the test case below.
>
> Ex: (output after this patch)but before this patch, below command is passing.
> /pg_restore  x1 -d postgres -j 10 -C --verbose --format=
> pg_restore: error: unrecognized archive format ""; please specify "c", "d", or "t"
>
> Here, I am attaching a patch which is fixing the same. I added 2 TAP tests also for invalid options.
>
> Note: We have 2 more options in pg_restore code which validate the option if arg has non zero length. I will prepare patches for both(--host and --port). We need to add some validate function also for both these options.
>
> --
> Thanks and Regards
> Mahendra Singh Thalor
> EnterpriseDB: http://www.enterprisedb.com

Hi all,
Here I am attaching a re-based patch.

I think we should sync behaviour with pg_dump and pg_restore. Please
review this patch and let me know feedback.

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


Attachments:

  [text/x-patch] v02-pg_restore-format-option-should-validate-all-values.patch (2.3K, 2-v02-pg_restore-format-option-should-validate-all-values.patch)
  download | inline diff:
From 8fa9f056d567ce350e88503c872e0267592100a0 Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Sun, 15 Mar 2026 09:42:52 +0530
Subject: [PATCH] pg_restore --format option should validate all values

With "pg_restore --format=", we are not giving any error because in code, we are checking
length of arg but pg_dump is troughing an error for the same option.

Ex: before this patch, below command was passing.
/pg_restore  x1 -d postgres -j 10 -C --verbose --format=

after patch:
/pg_restore  x1 -d postgres -j 10 -C --verbose --format=
pg_restore: error: unrecognized archive format ""; please specify "c", "d", or "t"

Note: pg_dump is already reporting an error in above case.
---
 src/bin/pg_dump/pg_restore.c   |  3 +--
 src/bin/pg_dump/t/001_basic.pl | 10 ++++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)
 mode change 100644 => 100755 src/bin/pg_dump/t/001_basic.pl

diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index fb44c0cfdfe..d755d3c72dc 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -225,8 +225,7 @@ main(int argc, char **argv)
 				opts->filename = pg_strdup(optarg);
 				break;
 			case 'F':
-				if (strlen(optarg) != 0)
-					opts->formatName = pg_strdup(optarg);
+				opts->formatName = pg_strdup(optarg);
 				break;
 			case 'g':
 				/* restore only global sql commands. */
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
old mode 100644
new mode 100755
index 2f5eb48e7b8..bf661910c66
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -201,6 +201,16 @@ command_fails_like(
 	qr/\Qpg_restore: error: unrecognized archive format "garbage";\E/,
 	'pg_dump: unrecognized archive format');
 
+command_fails_like(
+	[ 'pg_restore', '-f -', '--format='],
+	qr/\Qpg_restore: error: unrecognized archive format "";\E/,
+	'pg_dump: unrecognized archive format empty string');
+
+command_fails_like(
+	[ 'pg_restore', '-f -', '-F', 'p' ],
+	qr/\Qpg_restore: error: archive format "p" is not supported; please use psql\E/,
+	'pg_dump: unrecognized archive format p|plain');
+
 command_fails_like(
 	[ 'pg_dump', '--on-conflict-do-nothing' ],
 	qr/pg_dump: error: option --on-conflict-do-nothing requires option --inserts, --rows-per-insert, or --column-inserts/,
-- 
2.52.0



view thread (9+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: pg_restore --format= option(without any value) should report an error as pg_dump is reporting an error
  In-Reply-To: <CAKYtNApxyeeQbjxjFCgHwuv0T2TJvFaU6QSQaMU_yr1nP4Y8Fg@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox