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

On 14.09.26 17:45, Matthias van de Meent wrote:
>> + /*
>> + * 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).

I think this change, meaning

- if (strlen(extconditionarray[j]) > 0)
+ if (strlen(extconditionarray[j]) > 0 && !dopt->binary_upgrade)

might be an independent bug fix, but it's not a bug you'd normally hit.

You can reproduce this straightforwardly by making an extension with a
configuration table and some filter with some "built-in" configuration
rows, and then dumping it with pg_dump --binary-upgrade. Since in
binary upgrade mode, CREATE EXTENSION doesn't actually run the
extension's SQL install scripts, the "built-in" configuration rows won't
be created, and only the "user-created" rows. By skipping the filter in
binary upgrade mode, as proposed by the patch, you'll get both built-in
and user-created rows dumped correctly.

But pg_upgrade runs pg_dump with --binary-upgrade --no-data, so it
doesn't dump any configuration table rows either way. So this normal
course of action is not affected by this problem.

But I think this change could still be useful to make pg_dump's behavior
internally consistent. This might be helpful to make future changes or
alternative uses more robust.

Alternatively, we could prohibit combinations of --binary-upgrade with
options that result in dumping data (no option, --data-only, etc.).