Re: Non-text mode for pg_dumpall

On Tue, Dec 9, 2025 at 2:49 AM Mahendra Singh Thalor <mahi6run(at)gmail(dot)com> wrote:
>
>
> Here, I am attaching an updated patch for the review and testing. This
> can be applied on commit d0d0ba6cf66c4043501f6f7.
>

hi.

more comments about
v12_09122025-Non-text-modes-for-pg_dumpall-correspondingly-change.patch

+ In all other modes, <application>pg_dumpall</application>
first creates two files:
+ <filename>toc.dat/toc.dmp/toc.tar</filename> and
<filename>map.dat</filename>, in the directory
+ specified by <option>--file</option>.
+ The first file contains global data, such as roles and
tablespaces. The second
+ contains a mapping between database oids and names. These
files are used by
+ <application>pg_restore</application>. Data for individual
databases is placed in
+ <filename>databases</filename> subdirectory, named using the
database's <type>oid</type>.

I tried all these 3 formats, there is no "toc.dmp/toc.tar".
Am I missing something?

-
+ If format is given, then dump will be based on format, default plain.
<screen>
<prompt>$</prompt> <userinput>pg_dumpall &gt; db.out</userinput>
+</screen>
+
+<screen>
+<prompt>$</prompt> <userinput>pg_dumpall --format=d/a/c/p -f db.out</userinput>
</screen>

The text in the <screen> section should work correctly when pasted directly into
the terminal.
but ``pg_dumpall --format=d/a/c/p -f db.out``
will error out:
``pg_dumpall: error: unrecognized output format "d/a/c/p"; please
specify "c", "d", "p", or "t"``

PGresult *
-executeQuery(PGconn *conn, const char *query)
+executeQuery(PGconn *conn, const char *query, bool is_archive)
{
PGresult *res;

@@ -287,7 +287,8 @@ executeQuery(PGconn *conn, const char *query)
{
pg_log_error("query failed: %s", PQerrorMessage(conn));
pg_log_error_detail("Query was: %s", query);
- PQfinish(conn);
+ if (!is_archive)
+ PQfinish(conn);
exit_nicely(1);
}
It would be nice to add some comments explaining why we don't call
PQfinish for archive format.

+/*
+ * createOneArchiveEntry
+ *
+ * This creates one archive entry based on format.
+ */
+static void
+createOneArchiveEntry(const char *query, const char *tag)
+{
+ CatalogId nilCatalogId = {0, 0};
+ Assert(fout != NULL);
+
+ ArchiveEntry(fout,
+ nilCatalogId, /* catalog ID */
+ createDumpId(), /* dump ID */
+ ARCHIVE_OPTS(.tag = tag,
+ .description = tag,
+ .section = SECTION_PRE_DATA,
+ .createStmt = query));
+}
this is only used when archDumpFormat is not archNull.
comments can change to
"This creates one archive entry for non-text archive"

+static int
+restore_one_database(const char *inputFileSpec, RestoreOptions *opts,
+ int numWorkers, bool append_data, int num, bool globals_only)
I guess, "num" means number of databases, but the name is
"restore_one_database".
seems confusing. Similarly, I am confused by
restore_global_objects parameter "num".

+ pg_log_error("option %s must be specified when restoring an archive
created by pg_dumpall",
+ "-C/--create");
+ pg_log_error_hint("Try \"%s --help\" for more information.", progname);
+ pg_log_error_hint("Individual databases can be restored using their
specific archives.");
Here we report that --create must be specified.
The second pg_log_error_hint() message about restoring individual databases
seems unrelated to this requirement, and seems confusing in this context.

get_dbnames_list_to_restore
+ if (!conn && db_exclude_patterns.head != NULL)
+ pg_log_info("considering PATTERN as NAME for --exclude-database
option as no database connection while doing pg_restore");
is unreachable. because conn is always not NULL,
Since restore_all_databases "template1", template database "template1" is
undroppable, see ``dbcommands.c:1734``.

get_dbname_oid_list_from_mfile does not handle database names that contain
newline characters correctly.
For example:
CREATE DATABASE "test
\r";

I am unable to dump and restore a database with such a name.

--
jian
https://www.enterprisedb.com/