Re: pgsql: Non text modes for pg_dumpall, correspondingly change pg_restore - Mailing list pgsql-hackers

From Mahendra Singh Thalor
Subject Re: pgsql: Non text modes for pg_dumpall, correspondingly change pg_restore
Date
Msg-id CAKYtNAqPQJEGc5GSKiBEnCBCx1witrRauZ=2i0M4Fm4Rmug4_Q@mail.gmail.com
Whole thread Raw
In response to Re: pgsql: Non text modes for pg_dumpall, correspondingly change pg_restore  (Andrew Dunstan <andrew@dunslane.net>)
Responses Re: pgsql: Non text modes for pg_dumpall, correspondingly change pg_restore
Re: pgsql: Non text modes for pg_dumpall, correspondingly change pg_restore
List pgsql-hackers
On Mon, 14 Apr 2025 at 21:39, Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> On 2025-Apr-04, Andrew Dunstan wrote:
>
> > Non text modes for pg_dumpall, correspondingly change pg_restore
>
> I think the new oid_string_list stuff in this commit is unnecessary, and
> we can remove a bunch of lines by simplifying that to using
> SimplePtrList, as the attached illustrates.  (Perhaps the names of
> members of the proposed struct can be improved.)

Thanks Álvaro for the patch.

I took this patch and did some testing. Patch looks good to me. I was not able to use "git am" in my setup due to CR's in patch but no warning in the patch when I used "patch -p".

One review comment:
We are using FLEXIBLE_ARRAY_MEMBER for dbname. Can we use NAMEDATALEN? In code, many places we are using this hard coded value of 64 size for names.

On Tue, 15 Apr 2025 at 01:44, Andrew Dunstan <andrew@dunslane.net> wrote:
>
>
> On 2025-04-14 Mo 12:28 PM, Andrew Dunstan wrote:
> >
> >
> >>
> >> I'm also not sure about the double sscanf() business there ... There
> >> must be a better way to do this.
> >
> >
> >
> > Yes, probably. I'll look into that if you like.
> >
> >
> >
>
> something like this?
>
> cheers
>
>
> andrew
>
>
> --
> Andrew Dunstan
> EDB: https://www.enterprisedb.com

Thanks Andrew.

Your patch looks good to me. I took your patch and did 1-2 line adjustments as per below. Please have a look.

--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -1053,15 +1053,19 @@ get_dbname_oid_list_from_mfile(const char *dumpdirpath, SimpleOidStringList *dbn
        while ((fgets(line, MAXPGPATH, pfile)) != NULL)
        {
                Oid                     db_oid = InvalidOid;
-               char            db_oid_str[MAXPGPATH + 1] = "";
                char       *dbname;
+               char            *p = line;
 
                /* Extract dboid. */
                sscanf(line, "%u", &db_oid);
-               sscanf(line, "%20s", db_oid_str);
+
+               while(isdigit(*p))
+                       p++;
+
+               Assert(*p == ' ');
 
                /* dbname is the rest of the line */
-               dbname = line + strlen(db_oid_str) + 1;
+               dbname = ++p;
 
                /* Remove \n from dbname. */
                dbname[strlen(dbname) - 1] = '\0';

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

pgsql-hackers by date:

Previous
From: Nathan Bossart
Date:
Subject: Re: ci: Allow running mingw tests by default via environment variable
Next
From: Michael Paquier
Date:
Subject: Re: Add pg_get_injection_points() for information of injection points