MIME-Version: 1.0
References: 
 <CAKYtNApkh=Vy2DpNRCnEJmPpxNuksbAh_QBav=2fLmVjBhGwFw@mail.gmail.com>
 <CAKYtNApxyeeQbjxjFCgHwuv0T2TJvFaU6QSQaMU_yr1nP4Y8Fg@mail.gmail.com>
 <1d66a1e5-ce44-448b-8407-31d5ed9bc10d@dunslane.net>
In-Reply-To: <1d66a1e5-ce44-448b-8407-31d5ed9bc10d@dunslane.net>
From: Mahendra Singh Thalor <mahi6run@gmail.com>
Date: Sun, 15 Mar 2026 22:32:11 +0530
Message-ID: 
 <CAKYtNArQ-c8xfBaWOKBB6TPVM5Fd_d0v2UEpxk9f5BuKyrQYxQ@mail.gmail.com>
Subject: Re: pg_restore --format= option(without any value) should report an
 error as pg_dump is reporting an error
To: Andrew Dunstan <andrew@dunslane.net>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/mixed; boundary="000000000000e1c98e064d13108a"
Archived-At: 
 <https://www.postgresql.org/message-id/CAKYtNArQ-c8xfBaWOKBB6TPVM5Fd_d0v2UEpxk9f5BuKyrQYxQ%40mail.gmail.com>
Precedence: bulk

--000000000000e1c98e064d13108a
Content-Type: text/plain; charset="UTF-8"

On Sun, 15 Mar 2026 at 22:01, Andrew Dunstan <andrew@dunslane.net> wrote:
>
>
> On 2026-03-15 Su 12:18 AM, Mahendra Singh Thalor wrote:
>
> On Sun, 13 Apr 2025 at 18:32, Mahendra Singh Thalor <mahi6run@gmail.com> 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.
>
>
>
> Let's try to deal with all these in one hit, instead if piecemeal.
>
>
> cheers
>
>
> andrew
>
> --
> Andrew Dunstan
> EDB: https://www.enterprisedb.com

Thanks Srinath and Andrew for the review and feedback.

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. I think we don't need any test cases for host
and port.

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.

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

--000000000000e1c98e064d13108a
Content-Type: text/x-patch; charset="US-ASCII";
	name="v03-pg_restore-format-host-port-options-should-validate-all-values.patch"
Content-Disposition: attachment;
	filename="v03-pg_restore-format-host-port-options-should-validate-all-values.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mms041ur0>
X-Attachment-Id: f_mms041ur0

RnJvbSBjZTVlNDViNzQ0NTU2Yjc3MDI4ZjlkZmMzOThiMzMwZTkyZjBmNGFlIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBNYWhlbmRyYSBTaW5naCBUaGFsb3IgPG1haGk2cnVuQGdtYWls
LmNvbT4KRGF0ZTogU3VuLCAxNSBNYXIgMjAyNiAyMjoyMTo1NyArMDUzMApTdWJqZWN0OiBbUEFU
Q0hdIHBnX3Jlc3RvcmUgLUYvLS1mb3JtYXQsIC1oLy0taG9zdCwgLXAvLS1wb3J0ICBvcHRpb25z
IHNob3VsZAogdmFsaWRhdGUgdmFsdWVzIGluIGFsbCBjYXNlcwoKV2l0aCAicGdfcmVzdG9yZSAt
LWZvcm1hdD0iLCB3ZSBhcmUgbm90IGdpdmluZyBhbnkgZXJyb3IgYmVjYXVzZSBpbiBjb2RlLCB3
ZSBhcmUgY2hlY2tpbmcKbGVuZ3RoIG9mIGFyZyBidXQgcGdfZHVtcCBpcyB0cm91Z2hpbmcgYW4g
ZXJyb3IgZm9yIHRoZSBzYW1lIG9wdGlvbi4KCkV4OiBiZWZvcmUgdGhpcyBwYXRjaCwgYmVsb3cg
Y29tbWFuZCB3YXMgcGFzc2luZy4KL3BnX3Jlc3RvcmUgIHgxIC1kIHBvc3RncmVzIC1qIDEwIC1D
IC0tdmVyYm9zZSAtLWZvcm1hdD0KCmFmdGVyIHBhdGNoOgovcGdfcmVzdG9yZSAgeDEgLWQgcG9z
dGdyZXMgLWogMTAgLUMgLS12ZXJib3NlIC0tZm9ybWF0PQpwZ19yZXN0b3JlOiBlcnJvcjogdW5y
ZWNvZ25pemVkIGFyY2hpdmUgZm9ybWF0ICIiOyBwbGVhc2Ugc3BlY2lmeSAiYyIsICJkIiwgb3Ig
InQiCgpOb3RlOiBwZ19kdW1wIGFuZCBwZ19kdW1wYWxsIGFyZSBhbHJlYWR5IHJlcG9ydGluZyBh
biBlcnJvciBpbiBhYm92ZSBjYXNlLgoKc2FtZSBmaXggaXMgbmVlZGVkIGZvciAtaC8tLWhvc3Qs
IC1wLy0tcG9ydCBvcHRpb25zIGFsc28uCi0tLQogc3JjL2Jpbi9wZ19kdW1wL3BnX3Jlc3RvcmUu
YyAgIHwgIDkgKysrLS0tLS0tCiBzcmMvYmluL3BnX2R1bXAvdC8wMDFfYmFzaWMucGwgfCAxMCAr
KysrKysrKysrCiAyIGZpbGVzIGNoYW5nZWQsIDEzIGluc2VydGlvbnMoKyksIDYgZGVsZXRpb25z
KC0pCiBtb2RlIGNoYW5nZSAxMDA2NDQgPT4gMTAwNzU1IHNyYy9iaW4vcGdfZHVtcC90LzAwMV9i
YXNpYy5wbAoKZGlmZiAtLWdpdCBhL3NyYy9iaW4vcGdfZHVtcC9wZ19yZXN0b3JlLmMgYi9zcmMv
YmluL3BnX2R1bXAvcGdfcmVzdG9yZS5jCmluZGV4IDliNGIxNTFiMzE4Li5mYTVlMTM1NWU3MSAx
MDA2NDQKLS0tIGEvc3JjL2Jpbi9wZ19kdW1wL3BnX3Jlc3RvcmUuYworKysgYi9zcmMvYmluL3Bn
X2R1bXAvcGdfcmVzdG9yZS5jCkBAIC0yMjUsMTYgKzIyNSwxNCBAQCBtYWluKGludCBhcmdjLCBj
aGFyICoqYXJndikKIAkJCQlvcHRzLT5maWxlbmFtZSA9IHBnX3N0cmR1cChvcHRhcmcpOwogCQkJ
CWJyZWFrOwogCQkJY2FzZSAnRic6Ci0JCQkJaWYgKHN0cmxlbihvcHRhcmcpICE9IDApCi0JCQkJ
CW9wdHMtPmZvcm1hdE5hbWUgPSBwZ19zdHJkdXAob3B0YXJnKTsKKwkJCQlvcHRzLT5mb3JtYXRO
YW1lID0gcGdfc3RyZHVwKG9wdGFyZyk7CiAJCQkJYnJlYWs7CiAJCQljYXNlICdnJzoKIAkJCQkv
KiByZXN0b3JlIG9ubHkgZ2xvYmFsIHNxbCBjb21tYW5kcy4gKi8KIAkJCQlnbG9iYWxzX29ubHkg
PSB0cnVlOwogCQkJCWJyZWFrOwogCQkJY2FzZSAnaCc6Ci0JCQkJaWYgKHN0cmxlbihvcHRhcmcp
ICE9IDApCi0JCQkJCW9wdHMtPmNwYXJhbXMucGdob3N0ID0gcGdfc3RyZHVwKG9wdGFyZyk7CisJ
CQkJb3B0cy0+Y3BhcmFtcy5wZ2hvc3QgPSBwZ19zdHJkdXAob3B0YXJnKTsKIAkJCQlicmVhazsK
IAkJCWNhc2UgJ2onOgkJCS8qIG51bWJlciBvZiByZXN0b3JlIGpvYnMgKi8KIAkJCQlpZiAoIW9w
dGlvbl9wYXJzZV9pbnQob3B0YXJnLCAiLWovLS1qb2JzIiwgMSwKQEAgLTI2NCw4ICsyNjIsNyBA
QCBtYWluKGludCBhcmdjLCBjaGFyICoqYXJndikKIAkJCQlicmVhazsKIAogCQkJY2FzZSAncCc6
Ci0JCQkJaWYgKHN0cmxlbihvcHRhcmcpICE9IDApCi0JCQkJCW9wdHMtPmNwYXJhbXMucGdwb3J0
ID0gcGdfc3RyZHVwKG9wdGFyZyk7CisJCQkJb3B0cy0+Y3BhcmFtcy5wZ3BvcnQgPSBwZ19zdHJk
dXAob3B0YXJnKTsKIAkJCQlicmVhazsKIAkJCWNhc2UgJ1InOgogCQkJCS8qIG5vLW9wLCBzdGls
bCBhY2NlcHRlZCBmb3IgYmFja3dhcmRzIGNvbXBhdGliaWxpdHkgKi8KZGlmZiAtLWdpdCBhL3Ny
Yy9iaW4vcGdfZHVtcC90LzAwMV9iYXNpYy5wbCBiL3NyYy9iaW4vcGdfZHVtcC90LzAwMV9iYXNp
Yy5wbApvbGQgbW9kZSAxMDA2NDQKbmV3IG1vZGUgMTAwNzU1CmluZGV4IDJmNWViNDhlN2I4Li4z
OTE0ZmIxNThjMgotLS0gYS9zcmMvYmluL3BnX2R1bXAvdC8wMDFfYmFzaWMucGwKKysrIGIvc3Jj
L2Jpbi9wZ19kdW1wL3QvMDAxX2Jhc2ljLnBsCkBAIC0yMDEsNiArMjAxLDE2IEBAIGNvbW1hbmRf
ZmFpbHNfbGlrZSgKIAlxci9cUXBnX3Jlc3RvcmU6IGVycm9yOiB1bnJlY29nbml6ZWQgYXJjaGl2
ZSBmb3JtYXQgImdhcmJhZ2UiO1xFLywKIAkncGdfZHVtcDogdW5yZWNvZ25pemVkIGFyY2hpdmUg
Zm9ybWF0Jyk7CiAKK2NvbW1hbmRfZmFpbHNfbGlrZSgKKwlbICdwZ19yZXN0b3JlJywgJy1mIC0n
LCAnLS1mb3JtYXQ9J10sCisJcXIvXFFwZ19yZXN0b3JlOiBlcnJvcjogdW5yZWNvZ25pemVkIGFy
Y2hpdmUgZm9ybWF0ICIiO1xFLywKKwkncGdfcmVzdG9yZTogdW5yZWNvZ25pemVkIGFyY2hpdmUg
Zm9ybWF0IGVtcHR5IHN0cmluZycpOworCitjb21tYW5kX2ZhaWxzX2xpa2UoCisJWyAncGdfcmVz
dG9yZScsICctZiAtJywgJy1GJywgJ3AnIF0sCisJcXIvXFFwZ19yZXN0b3JlOiBlcnJvcjogYXJj
aGl2ZSBmb3JtYXQgInAiIGlzIG5vdCBzdXBwb3J0ZWQ7IHBsZWFzZSB1c2UgcHNxbFxFLywKKwkn
cGdfcmVzdG9yZTogdW5yZWNvZ25pemVkIGFyY2hpdmUgZm9ybWF0IHB8cGxhaW4nKTsKKwogY29t
bWFuZF9mYWlsc19saWtlKAogCVsgJ3BnX2R1bXAnLCAnLS1vbi1jb25mbGljdC1kby1ub3RoaW5n
JyBdLAogCXFyL3BnX2R1bXA6IGVycm9yOiBvcHRpb24gLS1vbi1jb25mbGljdC1kby1ub3RoaW5n
IHJlcXVpcmVzIG9wdGlvbiAtLWluc2VydHMsIC0tcm93cy1wZXItaW5zZXJ0LCBvciAtLWNvbHVt
bi1pbnNlcnRzLywKLS0gCjIuNTIuMAoK
--000000000000e1c98e064d13108a--