On 2025-03-27 Th 5:15 PM, Andrew Dunstan wrote:
>
> On 2025-03-19 We 2:41 AM, Mahendra Singh Thalor wrote:
>> On Wed, 12 Mar 2025 at 21:18, Andrew Dunstan <andrew@dunslane.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@alvh.no-ip.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).
>
>
>
OK, I have done that, so here is the result. The first two are you
original patches. patch 3 adds the new list type to fe-utils, and patch
4 contains my cleanups and use of the new list type. Apart from some
relatively minor cleanup, the one thing I would like to change is how
dumps are named. If we are producing tar or custom format dumps, I think
the file names should reflect that (oid.dmp and oid.tar rather than a
bare oid as the filename), and pg_restore should look for those. I'm
going to work on that tomorrow - I don't think it will be terribly
difficult.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
