Обсуждение: why -Fdance archive format option works with ./pg_restore but not with ./pg_dump?

On 2025-01-23 Th 3:18 AM, Srinath Reddy wrote:
> Hi,
>
> in pg_restore archive format option is parsed using only the first 
> character of the value but in pg_dump the archive format option's 
> value is compared with whole string using pg_strcasecmp is there any 
> specific reason?
>
>


`git blame` tells me that this switch statement goes back to 2001. 
Seeking a reason at this distance is unlikely to be productive. Probably 
we were not nearly as careful then as we are now about such things. Feel 
free to submit a patch tightening the test.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

On Fri, Jan 24, 2025 at 1:42 AM Andrew Dunstan <andrew@dunslane.net> wrote:

`git blame` tells me that this switch statement goes back to 2001.
Seeking a reason at this distance is unlikely to be productive. Probably
we were not nearly as careful then as we are now about such things. Feel
free to submit a patch tightening the test.

Hi Andrew,
thanks for the reply,here's the patch for the same. 

I don't think we need a new file for this. pg_backup_utils.c is already there for routines common to pg_restore and pg_dump.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Andrew Dunstan <andrew@dunslane.net> writes:
> I don't think we need a new file for this. pg_backup_utils.c is already 
> there for routines common to pg_restore and pg_dump.

I'm not even on board with having a new function, because I doubt
we should try to share this code in the first place.  Who's to
say that pg_dump and pg_restore must support exactly the same list
of formats?  For example, in the future we might decide that some
format is obsolete and desupport it in pg_dump, while continuing
to allow it for awhile in pg_restore for compatibility reasons.
A closer-to-home possibility is that the work to allow non-text
output from pg_dumpall will result in a format that pg_restore
can read but pg_dump (by itself) doesn't write.

So I'd just scrap pg_restore's parsing logic for this and replace it
in-place.  To the extent that that's copying and pasting stuff, fine.
It's not like there's no other duplicativeness in their switch-parsing
logic.

            regards, tom lane

On 2025-01-24 Fr 10:24 AM, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> I don't think we need a new file for this. pg_backup_utils.c is already
>> there for routines common to pg_restore and pg_dump.
> I'm not even on board with having a new function, because I doubt
> we should try to share this code in the first place.  Who's to
> say that pg_dump and pg_restore must support exactly the same list
> of formats?  For example, in the future we might decide that some
> format is obsolete and desupport it in pg_dump, while continuing
> to allow it for awhile in pg_restore for compatibility reasons.
> A closer-to-home possibility is that the work to allow non-text
> output from pg_dumpall will result in a format that pg_restore
> can read but pg_dump (by itself) doesn't write.
>
> So I'd just scrap pg_restore's parsing logic for this and replace it
> in-place.  To the extent that that's copying and pasting stuff, fine.
> It's not like there's no other duplicativeness in their switch-parsing
> logic.
>
>             



Fair point. Agreed.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com