Re: [PATCH] pg_dump: Restore extension config table data before user objects during pg_upgrade

Hi Jimmy,

On Mon, 6 Apr 2026 at 21:33, Jimmy Angelakos <jimmy(dot)angelakos(at)pgedge(dot)com> wrote:
>
> Hi Andrew,

Please don't top-post, but instead interleave responses. That'll help
anyone follow discussions on specific points in the thread.

> My opinion is that this is a bugfix rather than a feature: Rather than adding new capability it's fixing pg_upgrade's behaviour, because it currently fails in the described scenario (SRID-constrained columns). The new code path isn't user facing and it only fires during pg_upgrade's internal use of pg_dump --binary-upgrade.

The --binary-upgrade mode is user-accessible, so I don't think I agree
with your claim that it isn't user-facing.

Regardless, I think the new code should be built for future pg_dump
versions. A pg_dump can include extensions, and excluding the
extension config tables from the dump is likely to cause issues, so
let's make the main patch's code unconditional.

If you want to provide a backpatchable version that triggers only on
--binary-upgrade, I'm happy to review that, but please attach that
with a "nocfbot" file, so that we can avoid CI failing to apply the
patch to already-patched code.

> To address your feedback, please find attached v2 which:
> 1. Removes dumpExtensionData() and adds the handling for EXTENSION DATA object type to dumpTableData()
> 2. Adds test in test_pg_dump: we insert a row into the dumpable extension table, and we expect that the COPY appears in --binary-upgrade dumps.

This doesn't dedupe makeExtensionDataInfo()/makeTableDataInfo(), which
currently also has duplicative code.

> + /*
> + * For binary upgrades, dump extension config table data
> + * before user tables are created so it's available for
> + * validation (e.g. PostGIS SRIDs).
> + */

I'd reword this as "Dump extension config tables before user objects,
so that user objects can safely depend on those catalogs, if e.g. a
type of the extension defines typmod options in its catalogs (e.g.
PostGIS's geometry's SRIDs)."

> + /*
> + * For binary upgrade (DO_EXTENSION_DATA), don't apply
> + * the filter condition - we need ALL data since the
> + * extension won't populate built-in data in binary
> + * upgrade mode.
> + */

What does this comment mean? In a binary upgrade mode, we don't
filter table data because we're transfering the whole table-on-disk --
we never look at the contents of tables, only the RelFileLocator.
Adding more conditionals here is very confusing; I don't see any other
place where we do this.

Note that in a binary upgrade, CREATE EXTENSION doesn't actually run
the extension's SQL install scripts -- it copies the objects from the
old cluster's catalogs into the new cluster's catalogs, and re-links
the dependent catalog objects to the extension object; see the comment
in pg_dump.c's dumpExtension(), starting at line 11916 (current HEAD @
34fcd735f1).

Kind regards,

Matthias van de Meent
Databricks (https://www.databricks.com)