Re: Non-text mode for pg_dumpall

On 2025-03-19 We 2:41 AM, Mahendra Singh Thalor wrote:
> On Wed, 12 Mar 2025 at 21:18, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>>
>> On 2025-03-12 We 3:03 AM, jian he wrote:
>>> On Wed, Mar 12, 2025 at 1:06 AM Álvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>>>> Hello,
>>>>
>>>> On 2025-Mar-11, Mahendra Singh Thalor wrote:
>>>>
>>>>> In map.dat file, I tried to fix this issue by adding number of characters
>>>>> in dbname but as per code comments, as of now, we are not supporting \n\r
>>>>> in dbnames so i removed handling.
>>>>> I will do some more study to fix this issue.
>>>> Yeah, I think this is saying that you should not consider the contents
>>>> of map.dat as a shell string. After all, you're not going to _execute_
>>>> that file via the shell.
>>>>
>>>> Maybe for map.dat you need to escape such characters somehow, so that
>>>> they don't appear as literal newlines/carriage returns.
>>>>
>>> I am confused.
>>> currently pg_dumpall plain format will abort when encountering dbname
>>> containing newline.
>>> the left dumped plain file does not contain all the cluster databases data.
>>>
>>>
>>> if pg_dumpall non-text format aborts earlier,
>>> it's aligned with pg_dumpall plain format?
>>> it's also an improvement since aborts earlier, nothing will be dumped?
>>>
>>>
>>> am i missing something?
>>>
>>>
>> I think we should fix that.
>>
>> But for the current proposal, Álvaro and I were talking this morning,
>> and we thought the simplest thing here would be to have the one line
>> format and escape NL/CRs in the database name.
>>
>>
>> cheers
>>
> Okay. As per discussions, we will keep one line entry for each
> database into map.file.
>
> Thanks all for feedback and review.
>
> Here, I am attaching updated patches for review and testing. These
> patches can be applied on commit a6524105d20b.

I'm working through this patch set with a view to committing it.
Attached is some cleanup which is where I got to today, although there
is more to do. One thing I am wondering is why not put the
SimpleDatabaseOidList stuff in fe_utils/simle_list.{c,h} ? That's where
all the similar stuff belongs, and it feels strange to have this inline
in pg_restore.c. (I also don't like the name much - SimpleOidStringList
or maybe SimpleOidPlusStringList might be better).

cheers

andrew

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