On Sat, 29 Mar 2025 at 03:50, Andrew Dunstan <andrew@dunslane.net> wrote:
>
>
> 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.
>
Thanks Andrew.
Here, I am attaching a delta patch for oid.tar and oid.dmp format.
--
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com
