public inbox for [email protected]
help / color / mirror / Atom feedFrom: Mahendra Singh Thalor <[email protected]>
To: Andrew Dunstan <[email protected]>
Cc: jian he <[email protected]>
Cc: Álvaro Herrera <[email protected]>
Cc: Srinath Reddy <[email protected]>
Cc: [email protected]
Subject: Re: Non-text mode for pg_dumpall
Date: Sat, 29 Mar 2025 10:47:21 +0530
Message-ID: <CAKYtNAq8PeyK_bhqX7kR_WPp0x+Z3GAVPnqyBCxe5SVa1MRB7A@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAKYtNAr7ToCBSLO2Mm6NGFTZvbdTAFNgjv_YoRqJJghC3n1MiQ@mail.gmail.com>
<[email protected]>
<CACJufxG44zd+KV4B3_0d+Lf2fwnDDptX-DQRqn-o9ukmXEWeNw@mail.gmail.com>
<[email protected]>
<CAKYtNAoebXn-CC_cgv=-DxL1s7hPvx-1UW7CEfsiJZ8fbD6pjg@mail.gmail.com>
<[email protected]>
<[email protected]>
On Sat, 29 Mar 2025 at 03:50, Andrew Dunstan <[email protected]> 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 <[email protected]>
> >> wrote:
> >>>
> >>> On 2025-03-12 We 3:03 AM, jian he wrote:
> >>>> On Wed, Mar 12, 2025 at 1:06 AM Álvaro Herrera
> >>>> <[email protected]> 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
Attachments:
[application/octet-stream] delta-0001-pg_dumpall-dump-as-tar-and-dmp-file-for-file.patch (2.7K, 2-delta-0001-pg_dumpall-dump-as-tar-and-dmp-file-for-file.patch)
download | inline diff:
From b43ae117fe3809b82abb5bc89fc62d45a5707ff6 Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Sat, 29 Mar 2025 10:14:18 +0530
Subject: [PATCH] pg_dumpall - dump as .tar and .dmp file for tar and custom
format
---
src/bin/pg_dump/pg_dumpall.c | 7 ++++++-
src/bin/pg_dump/pg_restore.c | 22 +++++++++++++++++++++-
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 6aab1bfe831..12983d973be 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1685,7 +1685,12 @@ dumpDatabases(PGconn *conn, ArchiveFormat archDumpFormat)
*/
if (archDumpFormat != archNull)
{
- snprintf(dbfilepath, MAXPGPATH, "\"%s\"/\"%s\"", db_subdir, oid);
+ if (archDumpFormat == archCustom)
+ snprintf(dbfilepath, MAXPGPATH, "\"%s\"/\"%s\".dmp", db_subdir, oid);
+ else if (archDumpFormat == archTar)
+ snprintf(dbfilepath, MAXPGPATH, "\"%s\"/\"%s\".tar", db_subdir, oid);
+ else
+ snprintf(dbfilepath, MAXPGPATH, "\"%s\"/\"%s\"", db_subdir, oid);
/* Put one line entry for dboid and dbname in map file. */
fprintf(map_file, "%s %s\n", oid, dbname);
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 44a24791a6e..31cd9c84c5a 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -1164,6 +1164,8 @@ restoreAllDatabases(PGconn *conn, const char *dumpdirpath,
db_cell; db_cell = db_cell->next)
{
char subdirpath[MAXPGPATH];
+ char subdirdbpath[MAXPGPATH];
+ char dbfilename[MAXPGPATH];
int n_errors;
/* ignore dbs marked for skipping */
@@ -1180,7 +1182,25 @@ restoreAllDatabases(PGconn *conn, const char *dumpdirpath,
opts->cparams.override_dbname = NULL;
}
- snprintf(subdirpath, MAXPGPATH, "%s/databases/%u", dumpdirpath, db_cell->oid);
+ snprintf(subdirdbpath, MAXPGPATH, "%s/databases", dumpdirpath);
+
+ /*
+ * Validate database dump file. If there is .tar or .dmp file exist
+ * then consider particular file, otherwise just append dboid to the
+ * databases folder.
+ */
+ snprintf(dbfilename, MAXPGPATH, "%u.tar", db_cell->oid);
+ if (file_exists_in_directory(subdirdbpath, dbfilename))
+ snprintf(subdirpath, MAXPGPATH, "%s/databases/%u.tar", dumpdirpath, db_cell->oid);
+ else
+ {
+ snprintf(dbfilename, MAXPGPATH, "%u.dmp", db_cell->oid);
+
+ if (file_exists_in_directory(subdirdbpath, dbfilename))
+ snprintf(subdirpath, MAXPGPATH, "%s/databases/%u.dmp", dumpdirpath, db_cell->oid);
+ else
+ snprintf(subdirpath, MAXPGPATH, "%s/databases/%u", dumpdirpath, db_cell->oid);
+ }
pg_log_info("restoring database \"%s\"", db_cell->str);
--
2.39.3
view thread (99+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Non-text mode for pg_dumpall
In-Reply-To: <CAKYtNAq8PeyK_bhqX7kR_WPp0x+Z3GAVPnqyBCxe5SVa1MRB7A@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox