Re: Non-text mode for pg_dumpall

From: Mahendra Singh Thalor <mahi6run(at)gmail(dot)com>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: Srinath Reddy <srinath2133(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Non-text mode for pg_dumpall
Date: 2025-02-11 05:40:17
Message-ID: CAKYtNApSjr=etW+uVQ2KW0NcV=aZq-QdOd0jULync=84iDWr-w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks Jian.

On Tue, 4 Feb 2025 at 07:35, jian he <jian(dot)universality(at)gmail(dot)com> wrote:
>
> hi.
>
> just a quick response for v15.
>
> the pg_restore man page says option --list as "List the table of
> contents of the archive".
> but
> $BIN10/pg_restore --format=directory --list --file=1.sql dir10
> also output the contents of "global.dat", we should not output it.

I think we can add an error for --list option if used with the dump of
pg_dumpall. If a user wants to use --list option, then they can use a
single dump file.

>
> in restoreAllDatabases, we can do the following change:
> ```
> /* Open global.dat file and execute/append all the global sql commands. */
> if (!opts->tocSummary)
> process_global_sql_commands(conn, dumpdirpath, opts->filename);
> ```
>
>
> what should happen with
> $BIN10/pg_restore --format=directory --globals-only --verbose dir10 --list
>
> Should we error out saying "--globals-only" and "--list" are conflict options?
> if so then in main function we can do the following change:

Fixed.

>
> ```
> if (globals_only)
> {
> process_global_sql_commands(conn, inputFileSpec, opts->filename);
> if (conn)
> PQfinish(conn);
> pg_log_info("databases restoring is skipped as -g/--globals-only
> option is specified");
> }
> ```
>
>
> in restoreAllDatabases, if num_db_restore == 0, we will still call
> process_global_sql_commands.
> I am not sure this is what we expected.

This is correct. We should run global commands as we are dumping those
even if we don't dump any database.

Apart from these, I merged v15 delta to print db names. Either we can
print the db name or we can remove also but as of now, I merged delta
patch.

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

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

Attachment Content-Type Size
v16_pg_dumpall-with-non-text_format-11th_feb.patch application/octet-stream 75.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Zhijie Hou (Fujitsu) 2025-02-11 06:12:31 RE: Introduce XID age and inactive timeout based replication slot invalidation
Previous Message Ilia Evdokimov 2025-02-11 05:34:32 Re: 2025-02-13 release announcement draft