Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1vGdO8-002lAt-V1 for pgsql-hackers@arkaria.postgresql.org; Wed, 05 Nov 2025 13:17:24 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1vGdO6-002cNM-SX for pgsql-hackers@arkaria.postgresql.org; Wed, 05 Nov 2025 13:17:21 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1vGdO6-002cN8-Fk for pgsql-hackers@lists.postgresql.org; Wed, 05 Nov 2025 13:17:21 +0000 Received: from mail-ua1-x929.google.com ([2607:f8b0:4864:20::929]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.96) (envelope-from ) id 1vGdO1-0067i4-2X for pgsql-hackers@lists.postgresql.org; Wed, 05 Nov 2025 13:17:20 +0000 Received: by mail-ua1-x929.google.com with SMTP id a1e0cc1a2514c-932bc48197bso2758348241.0 for ; Wed, 05 Nov 2025 05:17:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb.com; s=google; t=1762348635; x=1762953435; darn=lists.postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=hU8IBjruM4YtrKyuhN+FSlDVx0IlIRqSxYZuyj+kSMI=; b=VzvQCeNy3P8+uoaYKgUjertq/3ctK/usD0l28AGZaN+aaHa7Xx9cBzWKi+aPZd4/3A dXZqdiHMsZ0Z1wGOm3rwwaVt4acOVHU9kyLE/OKxnvPEzRNtJHGZkgXGnSDjevM6Ak8G gn2NsSagU1kRJHDmW8DRt4ErFeXUOLMzMO0lDkSezKbjehai6wPHBkx0s7nGiHu5WtG6 qNeVPJnLBXbRqKzbRelZVIdFfmYgFmUHWmiwxOCB0ghdL2YU5w6R+J0hMxHje2nRNCD2 yeHrho1yEQP81HYkKV1BjkYvMfG7pCKPIjlGdfb40ZBs8AzVuD9PFrRueWkVHXu6pjlD poLA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1762348635; x=1762953435; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=hU8IBjruM4YtrKyuhN+FSlDVx0IlIRqSxYZuyj+kSMI=; b=MFtEHPKH1vCXkmjTXGSleqhRlA6dGm8FBGfx6YfRCL/rn1EXbiiZOYitxkTrD6z+Yj S1mxnuT/uUrwJ/yqO3PBvSM+zSrrW/TfZaSfti8WdLazAJtLcJCWLFUYBe/InnGbIFy8 5F4KNaZV1mUxGiE5UQopP+eJMmDNgTab7/v0I/1jaT28ocbJJ/92DD0bzfh3YACALY19 Io1oTcjBLKuVK/0rHrW/cnP0rw+cYTYEqbXFEfClotd2QDNdmtCUgd7nUstmIbE+rXIQ uje60tw1aKSGYxtQdvfbUMTaZoeMP+useb0nRsL/L+E/55azv8Rgp7XWUTaHENWflPU3 89XQ== X-Gm-Message-State: AOJu0Yy5a/dbsQ89NKuCb6vUHbc4KkSN9t37Q/v2d1gQIn4r/1wLXI5J ho2d9icLRW5Gle7Flgk0AAnHgnLO6N6/InmQeL65zWoS7igI/ggdDdSgfWAqucHF074M488INhn /DSpLBaJkdRLsKbxbtGCoAO6m+T9CqjtwP8O1eb4e X-Gm-Gg: ASbGncutG1lA1MNLgy0TPPNTnb3D97YaOkBRJRCKv8AXwCUGsHu8kWtOzKUw/4coBfb H0A92DVbqWhcCPfmU6L5pKaBtTT4Uc8M1uq6FX7pzrEWC6Bb8nk7swB8yuhfEWfl9gpHsYlabmQ yTj/fzKP9tf5Mb9RwsdW+cVCsQODjb53UAQxx9xXawSZsIdu5Lxs13k9G8Y6tuxO3nv+L7yAh0Y 8gIvCq6C0pqHBzPgwuALERM+DD5XetTnRaGmZmuFwjRwE6NCZiH7UAbuAk7sDM= X-Google-Smtp-Source: AGHT+IEl7cMIi6ZQ0v6s+nzRjPnZeZjIdMbfQDymapfEaClWD6OdbznirwymdmgMngYiW91mLQM6qpWiutgeI0YVP14= X-Received: by 2002:a05:6102:32cd:b0:5db:e64b:f68c with SMTP id ada2fe7eead31-5dd894ff6c1mr1050056137.33.1762348635009; Wed, 05 Nov 2025 05:17:15 -0800 (PST) MIME-Version: 1.0 References: <3f22a8bb-29e8-40cc-97a1-309181da2c13@dunslane.net> <20250722005339.ca.nmisch@google.com> <20250725162141.6f.nmisch@google.com> <2225040.1753477169@sss.pgh.pa.us> <20250727235628.e2.nmisch@google.com> <8f2ad50b-ebb7-4adc-997e-25e0ad96ff34@dunslane.net> <2bed001a-462c-42da-9a6b-3c7884502932@dunslane.net> <20250824010811.4d.nmisch@google.com> <82eb35b8-7f07-493b-b689-0934919e1dc3@dunslane.net> In-Reply-To: From: Vaibhav Dalvi Date: Wed, 5 Nov 2025 18:46:38 +0530 X-Gm-Features: AWmQ_bk4WZO6vP3eazJC-csX5jDNtaZFwyBwsfuPrwnEsVvLMYoc36K2kct7MRY Message-ID: Subject: Re: Non-text mode for pg_dumpall To: Mahendra Singh Thalor Cc: pgsql-hackers@lists.postgresql.org Content-Type: multipart/alternative; boundary="0000000000005c4df50642d8c4e3" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --0000000000005c4df50642d8c4e3 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Mahendra, Here are a few more comments following my review of the patch: ### 1\. Incorrect Comment for `-g` (globals-only) Option The comment for the `-g` case in the code states that it restores the `global.dat` file. However, in the non-text dump output, I only see the following files: `databases`, `map.dat`, and `toc.dat`. ```c + case 'g': + /* restore only global.dat file from directory */ + globals_only =3D true; + break; ``` Please update this comment to accurately reflect the file being restored (e.g., `toc.dat` or the global objects within the archive). ### 2\. Incorrect Order of `case` Statements in `pg_restore.c` The new `case 7` statement in `pg_restore.c` appears to be inserted before `case 6`, disrupting the numerical order. ```c + case 7: /* database patterns to skip */ + simple_string_list_append(&db_exclude_patterns, optarg); + break; case 6: opts->restrict_key =3D pg_strdup(optarg); ``` Please re-order the `case` statements so they follow ascending numerical order. ### 3\. Missing Example in SGML Documentation The SGML documentation for `pg_dumpall` is missing an explicit example demonstrating its use with non-text formats (e.g., directory format). It would be beneficial to include a clear example for this new feature. ### 4\. Cosmetic Issues Please address the following minor stylistic points: Please ensure the function signatures adhere to standard coding style, particularly for line wrapping. The following lines seem to have inconsistent indentation: ```c static int restore_global_objects(const char *inputFileSpec, RestoreOptions *opts, int numWorkers, bool append_data, int num, bool globals_only); static int restore_all_databases(const char *inputFileSpec, SimpleStringList db_exclude_patterns, RestoreOptions *opts, int numWorkers)= ; ``` Please fix instances where the 80-character line limit is crossed, such as in the example below: ```c n_errors =3D restore_one_database(subdirpath, opts, numWorkers, true, 1, false); ``` I believe this concludes my formal review. Thanks, Vaibhav Dalvi On Wed, Nov 5, 2025 at 12:29=E2=80=AFPM Vaibhav Dalvi < vaibhav.dalvi@enterprisedb.com> wrote: > Hi Mahendra, > > Thank you for the fix. Please find my further review comments below. > > ### Restrict-Key Option > > The `--restrict-key` option is currently being accepted by > `pg_dumpall` even when non-plain formats are specified, > which contradicts its intended use only with the plain format. > > For example: > > ``` > $ ./db/bin/pg_dump --format=3Dd -f testdump_dir --restrict-key=3DRESTRICT= _KEY > pg_dump: error: option --restrict-key can only be used with --format=3Dpl= ain > $ ./db/bin/pg_dumpall --format=3Dd -f testdump_dir > --restrict-key=3DRESTRICT_KEY > pg_dumpall: error: invalid restrict key > ``` > > I have attached a delta patch that addresses the issue with the > `--restrict-key` option. It would be beneficial to include a dedicated > test case for this check. > > ### Use of Dump Options Structure (dopt) > > Please ensure consistency by utilizing the main dump options > structure (`dopt`) instead of declaring and using individual variables > where the structure already provides fields. For example, the > `output_clean` variable seems redundant here: > > ```c > case 'c': > output_clean =3D true; > dopt.outputClean =3D 1; > break; > ``` > > In my attached delta file, I have replaced the unnecessary > `restrict_key` variable with `dopt.restrict_key`. > > ### Cosmetic Issues > > 1. Please review the spacing around the pointer: > ```c > + ((ArchiveHandle * )fout) ->connection =3D conn; > + ((ArchiveHandle * ) fout) -> public.numWorkers =3D 1; > ``` > 2. Please be consistent with the punctuation of single-line comments; > some end with a full stop (`.`) and others do not. > 3. In the SGML documentation changes, some new statements start > with one space, and others start with two. Please adhere to a single > standard for indentation across the patch. > > Regards, > Vaibhav > EnterpriseDB > > On Mon, Nov 3, 2025 at 5:24=E2=80=AFPM Mahendra Singh Thalor > wrote: > >> On Mon, 3 Nov 2025 at 12:06, Vaibhav Dalvi < >> vaibhav.dalvi@enterprisedb.com> wrote: >> > >> > Hi Mahendra, >> > >> > Thank you for your work on this feature. >> > I have just begun reviewing the latest patch and >> > encountered the following errors during the initial setup: >> > >> > ``` >> > $ ./db/bin/pg_restore testdump_dir -C -d postgres -F d -p 5556 >> > pg_restore: error: could not execute query: ERROR: syntax error at or >> near "\\" >> > LINE 1: \restrict aO9K1gzVZTlafidF5fWx8ADGzUnIiAcguFz5qskGaFDygTCjCj..= . >> > ^ >> > Command was: \restrict >> aO9K1gzVZTlafidF5fWx8ADGzUnIiAcguFz5qskGaFDygTCjCj9vg3Xxys1b3hb >> > >> > pg_restore: error: could not execute query: ERROR: syntax error at or >> near "\\" >> > LINE 1: \unrestrict aO9K1gzVZTlafidF5fWx8ADGzUnIiAcguFz5qskGaFDygTCj..= . >> > ^ >> > Command was: \unrestrict >> aO9K1gzVZTlafidF5fWx8ADGzUnIiAcguFz5qskGaFDygTCjCj9vg3Xxys1b3hb >> > >> > pg_restore: error: could not execute query: ERROR: syntax error at or >> near "\\" >> > LINE 1: \connect template1 >> > ^ >> > Command was: \connect template1 >> > >> > pg_restore: error: could not execute query: ERROR: syntax error at or >> near "\\" >> > LINE 1: \connect postgres >> > ^ >> > Command was: \connect postgres >> > ``` >> > To cross-check tried with plain dump(with pg_dumpall) and >> > restored(SQL file restore) without patch and didn't get above >> > connection errors. >> > >> > It appears there might be an issue with the dump file itself. >> > Please note that this is my first observation as I have just >> > started the review. I will continue with my assessment. >> > >> > Regards, >> > Vaibhav Dalvi >> > EnterpriseDB >> >> Thanks Vaibhav for the review. >> This change was added by me in v04. Only in the case of a file, we shoul= d >> restore these commands. Attached patch is fixing the same. >> >> If we dump and restore the same file with the same user, then we will ge= t >> an error of ROLE CREATE as the same role is already created. I think, >> either we can ignore this error, or we can keep it as a restore can be d= one >> with different users. >> >>> mst@localhost bin]$ ./pg_restore d1 -C -d postgres >>> pg_restore: error: could not execute query: ERROR: role "mst" already >>> exists >>> Command was: CREATE ROLE mst; >>> ALTER ROLE mst WITH SUPERUSER INHERIT CREATEROLE CREATEDB LOGIN >>> REPLICATION BYPASSRLS; >>> >>> >>> pg_restore: warning: errors ignored on restore: 1 >> >> >> >> > >> > On Fri, Oct 31, 2025 at 2:51=E2=80=AFPM Mahendra Singh Thalor < >> mahi6run@gmail.com> wrote: >> >> >> >> On Tue, 28 Oct 2025 at 11:32, Mahendra Singh Thalor < >> mahi6run@gmail.com> wrote: >> >> > >> >> > On Thu, 16 Oct 2025 at 16:24, Mahendra Singh Thalor < >> mahi6run@gmail.com> wrote: >> >> > > >> >> > > On Wed, 15 Oct 2025 at 23:05, Mahendra Singh Thalor < >> mahi6run@gmail.com> wrote: >> >> > > > >> >> > > > On Sun, 24 Aug 2025 at 22:12, Andrew Dunstan < >> andrew@dunslane.net> wrote: >> >> > > > > >> >> > > > > >> >> > > > > On 2025-08-23 Sa 9:08 PM, Noah Misch wrote: >> >> > > > > >> >> > > > > On Wed, Jul 30, 2025 at 02:51:59PM -0400, Andrew Dunstan wrot= e: >> >> > > > > >> >> > > > > OK, now that's reverted we should discuss how to proceed. I >> had two thoughts >> >> > > > > - we could use invent a JSON format for the globals, or we >> could just use >> >> > > > > the existing archive format. I think the archive format is >> pretty flexible, >> >> > > > > and should be able to accommodate this. The downside is it's >> not humanly >> >> > > > > readable. The upside is that we don't need to do anything >> special either to >> >> > > > > write it or parse it. >> >> > > > > >> >> > > > > I would first try to use the existing archiver API, because >> that makes it >> >> > > > > harder to miss bugs. Any tension between that API and >> pg_dumpall is likely to >> >> > > > > have corresponding tension on the pg_restore side. Resolving >> that tension >> >> > > > > will reveal much of the project's scope that remained hidden >> during the v18 >> >> > > > > attempt. Perhaps more important than that, using the archive= r >> API means >> >> > > > > future pg_dump and pg_restore options are more likely to >> cooperate properly >> >> > > > > with $SUBJECT. In other words, I want it to be hard to add >> pg_dump/pg_restore >> >> > > > > features that malfunction only for $SUBJECT archives. The >> strength of the >> >> > > > > archiver architecture shows in how rarely new features need >> format-specific >> >> > > > > logic and how rarely format-specific bugs get reported. We'v= e >> had little or >> >> > > > > no trouble with e.g. bugs that appear in -Fd but not in -Fc. >> >> > > > > >> >> > > > > >> >> > > > > Yeah, that's what we're going to try. >> >> > > > > >> >> > > > > >> >> > > > > cheers >> >> > > > > >> >> > > > > >> >> > > > > andrew >> >> > > > > >> >> > > > > -- >> >> > > > > Andrew Dunstan >> >> > > > > EDB: https://www.enterprisedb.com >> >> > > > >> >> > > > Thanks Andrew, Noah and all others for feedback. >> >> > > > >> >> > > > Based on the above suggestions and discussions, I removed sql >> commands >> >> > > > from the global.dat file. For global commands, now we are makin= g >> >> > > > toc.dat/toc.dmp/toc.tar file based on format specified and base= d >> on >> >> > > > format specified, we are making archive entries for these globa= l >> >> > > > commands. By this approach, we removed the hard-coded parsing >> part of >> >> > > > the global.dat file and we are able to skip DROP DATABASE with >> the >> >> > > > globals-only option. >> >> > > > >> >> > > > Here, I am attaching a patch for review, testing and feedback. >> This is >> >> > > > a WIP patch. I will do some more code cleanup and will add some >> more >> >> > > > comments also. Please review this and let me know design level >> >> > > > feedback. Thanks Tushar Ahuja for some internal testing and >> feedback. >> >> > > > >> >> > > >> >> > > Hi, >> >> > > Here, I am attaching an updated patch. In offline discussion, >> Andrew >> >> > > reported some test-case failures(Thanks Andrew). I fixed those. >> >> > > Please let me know feedback for the patch. >> >> > > >> >> > >> >> > Hi, >> >> > Here I am attaching a re-based patch as v02 was failing on head. >> >> > Thanks Tushar for the testing. >> >> > Please review this and let me know feedback. >> >> > >> >> >> >> Hi all, >> >> Here I am attaching an updated patch for review and testing. Based on >> >> some offline comments by Andrew, I did some code cleanup. >> >> Please consider this patch for feedback. >> >> >> >> -- >> >> Thanks and Regards >> >> Mahendra Singh Thalor >> >> EnterpriseDB: http://www.enterprisedb.com >> >> >> >> -- >> Thanks and Regards >> Mahendra Singh Thalor >> EnterpriseDB: http://www.enterprisedb.com >> > --0000000000005c4df50642d8c4e3 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Mahendra,

Here are a few = more comments following my review of the patch:

##= # 1\. Incorrect Comment for `-g` (globals-only) Option

=
The comment for the `-g` case in the code states that it restores the<= /div>
`global.dat` file. However, in the non-text dump output, I only s= ee the
following files: `databases`, `map.dat`, and `toc.dat`.

```c
+ case 'g':
+ = /* restore only global.dat file from directory */
+ globals_on= ly =3D true;
+ break;
```

P= lease update this comment to accurately reflect the file being restored
(e.g., `toc.dat` or the global objects within the archive).

### 2\. Incorrect Order of `case` Statements in `pg_restor= e.c`

The new `case 7` statement in `pg_restore.c` = appears to be
inserted before `case 6`, disrupting the numerical = order.

```c
+ case 7: /* database p= atterns to skip */
+ simple_string_list_append(&db_exclude= _patterns, optarg);
+ break;

case= 6:
opts->restrict_key =3D pg_strdup(optarg);
`= ``

Please re-order the `case` statements so they f= ollow ascending
numerical order.

### 3\.= Missing Example in SGML Documentation

The SGML do= cumentation for `pg_dumpall` is missing an explicit
example demon= strating its use with non-text formats (e.g., directory format).
= It would be beneficial to include a clear example for this new feature.

### 4\. Cosmetic Issues

Plea= se address the following minor stylistic points:

P= lease ensure the function signatures
adhere to standard coding st= yle, particularly for line wrapping.
The following lines seem to = have inconsistent indentation:

```c
= static int restore_global_objects(const char *inputFileSpec, RestoreOpt= ions *opts,
int numWorkers, bool append_data, int num, bool= globals_only);
static int restore_all_databases(const char *= inputFileSpec,
SimpleStringList db_exclude_patterns= , RestoreOptions *opts, int numWorkers);
```

Please fix instances where the 80-character line limit is
crossed, such as in the example below:

```c<= /div>
n_errors =3D restore_one_database(subdirpath, opts, numWorker= s, true, 1, false);
```

I believe th= is concludes my formal review.

Thanks,
V= aibhav Dalvi

On Wed, Nov 5, 2025 at 12:29=E2=80= =AFPM Vaibhav Dalvi <v= aibhav.dalvi@enterprisedb.com> wrote:
Hi Mahendra,
<= br>
Thank you for the fix. Please find my further review comments= below.

### Restrict-Key Option

The `--restrict-key` option is currently being accepted by
`pg_dumpall` even when non-plain formats are specified,
which co= ntradicts its intended use only with the plain format.

=
For example:

```
$ ./db/bin/pg_dump= --format=3Dd -f testdump_dir --restrict-key=3DRESTRICT_KEY
pg_du= mp: error: option --restrict-key can only be used with --format=3Dplain
$ ./db/bin/pg_dumpall --format=3Dd -f testdump_dir --restrict-key=3D= RESTRICT_KEY
pg_dumpall: error: invalid restrict key
``= `

I have attached a delta patch that addresses the= issue with the
`--restrict-key` option. It would be beneficial t= o include a dedicated
test case for this check.

### Use of Dump Options Structure (dopt)

P= lease ensure consistency by utilizing the main dump options
struc= ture (`dopt`) instead of declaring and using individual variables
where the structure already provides fields. For example, the
`o= utput_clean` variable seems redundant here:

```c
case 'c':
output_clean =3D true;
= dopt.outputClean =3D 1;
break;
```
In my attached delta file, I have replaced the unnecessary
`restrict_key` variable with `dopt.restrict_key`.

### Cosmetic Issues

1. Please review the spa= cing around the pointer:
```c
+ ((ArchiveHandl= e * )fout) ->connection =3D conn;
+ ((ArchiveHandle * ) f= out) -> public.numWorkers =3D 1;
```
2. Please b= e consistent with the punctuation of single-line comments;=C2=A0
= =C2=A0 =C2=A0 some end with a full stop (`.`) and others do not.
= 3. In the SGML documentation changes, some new statements start
= =C2=A0 =C2=A0 with one space, and others start with two. Please adhere to a= single
=C2=A0 =C2=A0 standard for indentation across the patch.<= /div>

Regards,
Vaibhav
EnterpriseDB<= /div>

On Mon, Nov 3, 2025 at 5:24=E2=80=AFPM Mahendra Singh Thalor <mahi6run@gmail.com&g= t; wrote:
On Mon, 3 Nov 2025 at 12:06, Vaibhav Dalvi <vaibhav.dalvi@enterprise= db.com> wrote:
>
> Hi Mahendra,
>
> Thank yo= u for your work on this feature.
> I have just begun reviewing the la= test patch and
> encountered the following errors during the initial = setup:
>
> ```
> $ ./db/bin/pg_restore testdump_dir -C -d= postgres -F d -p 5556
> pg_restore: error: could not execute query: = ERROR: syntax error at or near "\\"
> LINE 1: \restrict aO9= K1gzVZTlafidF5fWx8ADGzUnIiAcguFz5qskGaFDygTCjCj...
> ^
> Comman= d was: \restrict aO9K1gzVZTlafidF5fWx8ADGzUnIiAcguFz5qskGaFDygTCjCj9vg3Xxys= 1b3hb
>
> pg_restore: error: could not execute query: ERROR: sy= ntax error at or near "\\"
> LINE 1: \unrestrict aO9K1gzVZT= lafidF5fWx8ADGzUnIiAcguFz5qskGaFDygTCj...
> ^
> Command was: \u= nrestrict aO9K1gzVZTlafidF5fWx8ADGzUnIiAcguFz5qskGaFDygTCjCj9vg3Xxys1b3hb>
> pg_restore: error: could not execute query: ERROR: syntax er= ror at or near "\\"
> LINE 1: \connect template1
> ^<= br>> Command was: \connect template1
>
> pg_restore: error: = could not execute query: ERROR: syntax error at or near "\\"
&= gt; LINE 1: \connect postgres
> ^
> Command was: \connect postg= res
> ```
> To cross-check tried with plain dump(with pg_dumpal= l) and
> =C2=A0restored(SQL file restore) without patch and didn'= t get above
> connection errors.
>
> It appears there mig= ht be an issue with the dump file itself.
> Please note that this is = my first observation as I have just
> started the review. I will cont= inue with my assessment.
>
> Regards,
> Vaibhav Dalvi
= > EnterpriseDB

Thanks Vaibhav for the review.
This change was = added by me in v04. Only in the case of a file, we should restore these com= mands. Attached patch is fixing the same.

If we dump and restore the= same file with the same user, then we will get an error of ROLE CREATE as = the same role is already created. I think, either we can ignore this error,= or we can keep it as a restore can be done with different users.
mst@localhost bin]$ ./pg_restor= e d1 =C2=A0-C -d postgres
pg_restore: error: could not execute query: ER= ROR: =C2=A0role "mst" already exists
Command was: CREATE ROLE = mst;
ALTER ROLE mst WITH SUPERUSER INHERIT CREATEROLE CREATEDB LOGIN REP= LICATION BYPASSRLS;


pg_restore: warning: errors ignored on resto= re: 1


>
> On Fri, Oct 31, 2025 at 2:51=E2= =80=AFPM Mahendra Singh Thalor <mahi6run@gmail.com> wrote:
>>
>> O= n Tue, 28 Oct 2025 at 11:32, Mahendra Singh Thalor <mahi6run@gmail.com> wrote:
&g= t;> >
>> > On Thu, 16 Oct 2025 at 16:24, Mahendra Singh T= halor <mahi6run@= gmail.com> wrote:
>> > >
>> > > On Wed= , 15 Oct 2025 at 23:05, Mahendra Singh Thalor <mahi6run@gmail.com> wrote:
>>= ; > > >
>> > > > On Sun, 24 Aug 2025 at 22:12, A= ndrew Dunstan <= andrew@dunslane.net> wrote:
>> > > > >
>&= gt; > > > >
>> > > > > On 2025-08-23 Sa 9:= 08 PM, Noah Misch wrote:
>> > > > >
>> > &= gt; > > On Wed, Jul 30, 2025 at 02:51:59PM -0400, Andrew Dunstan wrot= e:
>> > > > >
>> > > > > OK, now = that's reverted we should discuss how to proceed. I had two thoughts>> > > > > - we could use invent a JSON format for the g= lobals, or we could just use
>> > > > > the existing a= rchive format. I think the archive format is pretty flexible,
>> &= gt; > > > and should be able to accommodate this. The downside is = it's not humanly
>> > > > > readable. The upside i= s that we don't need to do anything special either to
>> > = > > > write it or parse it.
>> > > > >
>= ;> > > > > I would first try to use the existing archiver AP= I, because that makes it
>> > > > > harder to miss bug= s.=C2=A0 Any tension between that API and pg_dumpall is likely to
>&g= t; > > > > have corresponding tension on the pg_restore side.= =C2=A0 Resolving that tension
>> > > > > will reveal m= uch of the project's scope that remained hidden during the v18
>&= gt; > > > > attempt.=C2=A0 Perhaps more important than that, us= ing the archiver API means
>> > > > > future pg_dump a= nd pg_restore options are more likely to cooperate properly
>> >= ; > > > with $SUBJECT.=C2=A0 In other words, I want it to be hard = to add pg_dump/pg_restore
>> > > > > features that mal= function only for $SUBJECT archives.=C2=A0 The strength of the
>> = > > > > archiver architecture shows in how rarely new features = need format-specific
>> > > > > logic and how rarely f= ormat-specific bugs get reported.=C2=A0 We've had little or
>>= > > > > no trouble with e.g. bugs that appear in -Fd but not i= n -Fc.
>> > > > >
>> > > > >
&= gt;> > > > > Yeah, that's what we're going to try.>> > > > >
>> > > > >
>> = > > > > cheers
>> > > > >
>> >= > > >
>> > > > > andrew
>> > >= ; > >
>> > > > > --
>> > > > &= gt; Andrew Dunstan
>> > > > > EDB: https://www.enterprisedb.com>> > > >
>> > > > Thanks Andrew, Noah and= all others for feedback.
>> > > >
>> > > = > Based on the above suggestions and discussions, I removed sql commands=
>> > > > from the global.dat file. For global commands, = now we are making
>> > > > toc.dat/toc.dmp/toc.tar file b= ased on format specified and based on
>> > > > format spe= cified, we are making archive entries for these global
>> > >= ; > commands. By this approach, we removed the hard-coded parsing part o= f
>> > > > the global.dat file and we are able to skip DR= OP DATABASE with the
>> > > > globals-only option.
>= ;> > > >
>> > > > Here, I am attaching a patc= h for review, testing and feedback. This is
>> > > > a WI= P patch. I will do some more code cleanup and will add some more
>>= ; > > > comments also. Please review this and let me know design l= evel
>> > > > feedback. Thanks Tushar Ahuja for some inte= rnal testing and feedback.
>> > > >
>> > >=
>> > > Hi,
>> > > Here, I am attaching an up= dated patch. In offline discussion, Andrew
>> > > reported s= ome test-case failures(Thanks Andrew). I fixed those.
>> > >= Please let me know feedback for the patch.
>> > >
>&g= t; >
>> > Hi,
>> > Here I am attaching a re-base= d patch as v02 was failing on head.
>> > Thanks Tushar for the = testing.
>> > Please review this and let me know feedback.
&= gt;> >
>>
>> Hi all,
>> Here I am attachin= g an updated patch for review and testing. Based on
>> some offlin= e comments by Andrew, I did some code cleanup.
>> Please consider = this patch for feedback.
>>
>> --
>> Thanks and = Regards
>> Mahendra Singh Thalor
>> EnterpriseDB: http://www.enterprisedb.= com



--
Thanks and Regards
Mahendra Singh ThalorEnterpriseDB: h= ttp://www.enterprisedb.com
--0000000000005c4df50642d8c4e3--