MIME-Version: 1.0
From: Mahendra Singh Thalor <mahi6run@gmail.com>
Date: Sun, 13 Apr 2025 18:32:01 +0530
Message-ID: 
 <CAKYtNApkh=Vy2DpNRCnEJmPpxNuksbAh_QBav=2fLmVjBhGwFw@mail.gmail.com>
Subject: pg_restore --format= option(without any value) should report an error
 as pg_dump is reporting an error
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/mixed; boundary="0000000000004706730632a88b2a"
Archived-At: 
 <https://www.postgresql.org/message-id/CAKYtNApkh%3DVy2DpNRCnEJmPpxNuksbAh_QBav%3D2fLmVjBhGwFw%40mail.gmail.com>
Precedence: bulk

--0000000000004706730632a88b2a
Content-Type: multipart/alternative; boundary="0000000000004706710632a88b28"

--0000000000004706710632a88b28
Content-Type: text/plain; charset="UTF-8"

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

--0000000000004706710632a88b28
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div>Hi,</div><div>With &quot;pg_restore --format=3D&quot;=
, we are not giving any error because=C2=A0in code, we are checking length =
of arg but pg_dump is reporting an error for the same option.</div><div><br=
></div><div>For the consistency=C2=A0purpose, pg_dump and pg_restore both s=
hould report an error for the test case below.</div><div><br></div><div><b>=
Ex: (output after this patch)but before this patch, below command is passin=
g.</b></div><div>/pg_restore =C2=A0x1 -d postgres -j 10 -C --verbose --form=
at=3D<br>pg_restore: error: unrecognized archive format &quot;&quot;; pleas=
e specify &quot;c&quot;, &quot;d&quot;, or &quot;t&quot;</div><div><br></di=
v><div>Here, I am attaching a patch which is fixing the same. I added 2 TAP=
 tests also for invalid options.</div><div><br></div><div><b>Note:</b>=C2=
=A0We have 2 more options in pg_restore code which validate the option if a=
rg 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.</div><d=
iv><br></div><span class=3D"gmail_signature_prefix">-- </span><br><div dir=
=3D"ltr" class=3D"gmail_signature" data-smartmail=3D"gmail_signature">Thank=
s and Regards<br>Mahendra Singh Thalor<br>EnterpriseDB: <a href=3D"http://w=
ww.enterprisedb.com" target=3D"_blank">http://www.enterprisedb.com</a></div=
></div>

--0000000000004706710632a88b28--

--0000000000004706730632a88b2a
Content-Type: application/octet-stream;
	name="v01-pg_restore-format-option-should-validate-all-values.patch"
Content-Disposition: attachment;
	filename="v01-pg_restore-format-option-should-validate-all-values.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_m9fnl3fz0>
X-Attachment-Id: f_m9fnl3fz0

RnJvbSBiNTUzNDA1ZTUwYjI1ZmZjZjlkNDA4ZTIxZDFmOTI2YzY0NTI3NDhkIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBNYWhlbmRyYSBTaW5naCBUaGFsb3IgPG1haGk2cnVuQGdtYWls
LmNvbT4KRGF0ZTogU3VuLCAxMyBBcHIgMjAyNSAxOToxNzo1NyArMDUzMApTdWJqZWN0OiBbUEFU
Q0hdIHBnX3Jlc3RvcmUgLS1mb3JtYXQgb3B0aW9uIHNob3VsZCB2YWxpZGF0ZSBhbGwgdmFsdWVz
CgpXaXRoICJwZ19yZXN0b3JlIC0tZm9ybWF0PSIsIHdlIGFyZSBub3QgZ2l2aW5nIGFueSBlcnJv
ciBiZWNhdXNlIGluIGNvZGUsIHdlIGFyZSBjaGVja2luZwpsZW5ndGggb2YgYXJnIGJ1dCBwZ19k
dW1wIGlzIHRyb3VnaGluZyBhbiBlcnJvciBmb3IgdGhlIHNhbWUgb3B0aW9uLgoKRXg6IChhZnRl
ciB0aGlzIHBhdGNoKS0tIGJlZm9yZSB0aGlzIHBhdGNoLCBiZWxvdyBjb21tYW5kIGlzIHBhc3Np
bmcuCi9wZ19yZXN0b3JlICB4MSAtZCBwb3N0Z3JlcyAtaiAxMCAtQyAtLXZlcmJvc2UgLS1mb3Jt
YXQ9CnBnX3Jlc3RvcmU6IGVycm9yOiB1bnJlY29nbml6ZWQgYXJjaGl2ZSBmb3JtYXQgIiI7IHBs
ZWFzZSBzcGVjaWZ5ICJjIiwgImQiLCBvciAidCIKLS0tCiBzcmMvYmluL3BnX2R1bXAvcGdfcmVz
dG9yZS5jICAgfCAgMyArLS0KIHNyYy9iaW4vcGdfZHVtcC90LzAwMV9iYXNpYy5wbCB8IDEwICsr
KysrKysrKysKIDIgZmlsZXMgY2hhbmdlZCwgMTEgaW5zZXJ0aW9ucygrKSwgMiBkZWxldGlvbnMo
LSkKIG1vZGUgY2hhbmdlIDEwMDY0NCA9PiAxMDA3NTUgc3JjL2Jpbi9wZ19kdW1wL3QvMDAxX2Jh
c2ljLnBsCgpkaWZmIC0tZ2l0IGEvc3JjL2Jpbi9wZ19kdW1wL3BnX3Jlc3RvcmUuYyBiL3NyYy9i
aW4vcGdfZHVtcC9wZ19yZXN0b3JlLmMKaW5kZXggZmY0YmIzMjBmYzkuLmM4NmQ1OTNkZjkyIDEw
MDY0NAotLS0gYS9zcmMvYmluL3BnX2R1bXAvcGdfcmVzdG9yZS5jCisrKyBiL3NyYy9iaW4vcGdf
ZHVtcC9wZ19yZXN0b3JlLmMKQEAgLTIxNyw4ICsyMTcsNyBAQCBtYWluKGludCBhcmdjLCBjaGFy
ICoqYXJndikKIAkJCQlvcHRzLT5maWxlbmFtZSA9IHBnX3N0cmR1cChvcHRhcmcpOwogCQkJCWJy
ZWFrOwogCQkJY2FzZSAnRic6Ci0JCQkJaWYgKHN0cmxlbihvcHRhcmcpICE9IDApCi0JCQkJCW9w
dHMtPmZvcm1hdE5hbWUgPSBwZ19zdHJkdXAob3B0YXJnKTsKKwkJCQlvcHRzLT5mb3JtYXROYW1l
ID0gcGdfc3RyZHVwKG9wdGFyZyk7CiAJCQkJYnJlYWs7CiAJCQljYXNlICdnJzoKIAkJCQkvKiBy
ZXN0b3JlIG9ubHkgZ2xvYmFsLmRhdCBmaWxlIGZyb20gZGlyZWN0b3J5ICovCmRpZmYgLS1naXQg
YS9zcmMvYmluL3BnX2R1bXAvdC8wMDFfYmFzaWMucGwgYi9zcmMvYmluL3BnX2R1bXAvdC8wMDFf
YmFzaWMucGwKb2xkIG1vZGUgMTAwNjQ0Cm5ldyBtb2RlIDEwMDc1NQppbmRleCA4NGNhMjVlMTdk
Ni4uNjdiOTdlYzViOTIKLS0tIGEvc3JjL2Jpbi9wZ19kdW1wL3QvMDAxX2Jhc2ljLnBsCisrKyBi
L3NyYy9iaW4vcGdfZHVtcC90LzAwMV9iYXNpYy5wbApAQCAtMjAxLDYgKzIwMSwxNiBAQCBjb21t
YW5kX2ZhaWxzX2xpa2UoCiAJcXIvXFFwZ19yZXN0b3JlOiBlcnJvcjogdW5yZWNvZ25pemVkIGFy
Y2hpdmUgZm9ybWF0ICJnYXJiYWdlIjtcRS8sCiAJJ3BnX2R1bXA6IHVucmVjb2duaXplZCBhcmNo
aXZlIGZvcm1hdCcpOwogCitjb21tYW5kX2ZhaWxzX2xpa2UoCisJWyAncGdfcmVzdG9yZScsICct
ZiAtJywgJy0tZm9ybWF0PSddLAorCXFyL1xRcGdfcmVzdG9yZTogZXJyb3I6IHVucmVjb2duaXpl
ZCBhcmNoaXZlIGZvcm1hdCAiIjtcRS8sCisJJ3BnX2R1bXA6IHVucmVjb2duaXplZCBhcmNoaXZl
IGZvcm1hdCBlbXB0eSBzdHJpbmcnKTsKKworY29tbWFuZF9mYWlsc19saWtlKAorCVsgJ3BnX3Jl
c3RvcmUnLCAnLWYgLScsICctRicsICdwJyBdLAorCXFyL1xRcGdfcmVzdG9yZTogZXJyb3I6IGFy
Y2hpdmUgZm9ybWF0ICJwIiBpcyBub3Qgc3VwcG9ydGVkOyBwbGVhc2UgdXNlIHBzcWxcRS8sCisJ
J3BnX2R1bXA6IHVucmVjb2duaXplZCBhcmNoaXZlIGZvcm1hdCBwfHBsYWluJyk7CisKIGNvbW1h
bmRfZmFpbHNfbGlrZSgKIAlbICdwZ19kdW1wJywgJy0tb24tY29uZmxpY3QtZG8tbm90aGluZycg
XSwKIAlxci9wZ19kdW1wOiBlcnJvcjogb3B0aW9uIC0tb24tY29uZmxpY3QtZG8tbm90aGluZyBy
ZXF1aXJlcyBvcHRpb24gLS1pbnNlcnRzLCAtLXJvd3MtcGVyLWluc2VydCwgb3IgLS1jb2x1bW4t
aW5zZXJ0cy8sCi0tIAoyLjM5LjMKCg==
--0000000000004706730632a88b2a--