Re: Non-text mode for pg_dumpall

On Thu, 9 Jan 2025 at 08:11, Mahendra Singh Thalor <mahi6run(at)gmail(dot)com>
wrote:
>
> On Thu, 9 Jan 2025 at 02:30, Guillaume Lelarge <guillaume(at)lelarge(dot)info>
wrote:
> >
> > Hi,
> >
> > Le mer. 8 janv. 2025 à 17:41, Mahendra Singh Thalor <mahi6run(at)gmail(dot)com>
a écrit :
> >>
> >> On Wed, 8 Jan 2025 at 20:07, Mahendra Singh Thalor <mahi6run(at)gmail(dot)com>
wrote:
> >> >
> >> > Hi all,
> >> >
> >> > On Wed, 8 Jan 2025 at 00:34, Mahendra Singh Thalor <
mahi6run(at)gmail(dot)com> wrote:
> >> > >
> >> > > On Mon, 6 Jan 2025 at 23:05, Nathan Bossart <
nathandbossart(at)gmail(dot)com> wrote:
> >> > > >
> >> > > > On Thu, Jan 02, 2025 at 02:05:13AM +0530, Mahendra Singh Thalor
wrote:
> >> > > > > Here, I am attaching an updated patch. I fixed some bugs of
v01 patch and
> >> > > > > did some code cleanup also.
> >> > > >
> >> > > > Thank you for picking this up! I started to review it, but the
> >> > > > documentation changes didn't build, and a few tests in
check-world are
> >> > > > failing. Would you mind resolving those issues? Also, if you
haven't
> >> > > > already, please add an entry to the next commitfest [0] to
ensure that 1)
> >> > > > this feature is tracked and 2) the automated tests will run.
> >> > >
> >> > > Thanks Nathan for the quick response.
> >> > >
> >> > > I fixed bugs of documentation changes and check-world in the
latest patch. Now docs are building and check-world is passing.
> >> > >
> >> > > I added entry into commitfest for this patch.[0]
> >> > >
> >> > > >
> >> > > > + if (dbfile)
> >> > > > + {
> >> > > > + printfPQExpBuffer(&cmd, "\"%s\" %s %s",
pg_dump_bin,
> >> > > > + dbfile,
create_opts);
> >> > > > + appendPQExpBufferStr(&cmd, " -F d ");
> >> > > > + }
> >> > > >
> >> > > > Have you given any thought to allowing a directory of custom
format files,
> >> > > > as discussed upthread [1]? Perhaps that is better handled as a
follow-up
> >> > > > patch, but it'd be good to understand the plan, anyway.
> >> > >
> >> > > I will make these changes and will test. I will update my findings
after doing some testing.
> >> >
> >> > In the latest patch, I added dump and restoring for
directory/custom/tar/plain formats. Please consider this patch for review
and testing.
> >> >
> >> > Design:
> >> > When we give --format=d|c|t then we are dumping all global sql
commands in global.dat in plain sql format and we are making a map.dat file
with dbname and dboid. For each database, we are making separate
subdirectory with dboid under databases directory and dumping as per
archive format(d|c|t).
> >> > While restoring, first we are restoring all global sql commands from
global.dat and then we are restoring one by one all databases. As we are
supporting --exclude-database with pg_dumpall, the same we are supporting
with pg_restore also to skip restoring on some specified database patterns.
> >> > If we want to restore a single database, then we can specided
particular subdirectory from the databases folder. To get file name, we
refer dbname into map.file.
> >> >
> >> > TODO: Now I will work on test cases for these new added options to
the pg_dumpall and pg_restore.
> >> >
> >> > Here, I am attaching the v04 patch for testing and review.
> >>
> >> Sorry. My mistake.
> >> v04 was the delta patch on the top of v03.
> >>
> >> Here, I am attaching the v05 patch for testing and review.
> >>
> >
> > Just FWIW, I did a quick test tonight. It applies cleanly, compiles OK.
I did a dump:
>
> Thanks for testing and review.
>
> >
> > $ pg_dumpall -Fd -f dir
> >
> > and then a restore (after dropping the databases I had):
> >
> > $ pg_restore -Cd postgres -v dir
> >
> > It worked really well. That's great.
> >
> > Quick thing to fix: you've got this error message:
> > pg_restore: error: -d/--dbanme should be given when using archive dump
of pg_dumpall
> >
> > I guess it is --dbname, rather than --dbanme.
>
> Fixed.
>
> >
> > Of course, it needs much more testing, but this feature would be great
to have. Thanks for working on this!
> >
> >
>
> Apart from above typo, I fixed some review comments those I received
> from Andrew in offline discussion. Thanks Andrew for the quick review.
>
> Here, I am attaching an updated patch for review and testing.
>
Hi all,

Based on some testing(dump was shared by Andrew, Thanks Andrew), I fixed
some more bugs in the attached patch.
There are some open points for this patch. I will put those points in
follow-up patches also.

*Point 1*: With pg_dumpall, we have option --exclude-database=PATTERN, and
there we are validating this PATTERN by server because
we have connection but in pg_restore, we don't have some db connection in
some case so how to handle these patterns? or should we use
only NAMES for --exclude-database ?

*Point 2*:
For each database, we are registering entry to on_exit_nicely array due AH
entry but max size of array is MAX_ON_EXIT_NICELY=20,
so after 20 db restoring, we are getting fatal so either my code needs to
reset this array or do we need to increase array size?

Here, I am attaching an updated patch for review and testing.

--
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com