Re: pg_dump crash on identity sequence with not loaded attributes

On Tue, 10 Dec 2024 at 22:15, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Yeah. After chewing on this for awhile, I think the cleanest solution
> is say that b965f2617 was just wrong, and we should revert it in
> favor of adopting this logic:
>
> if (seqinfo->is_identity_sequence)
> seqinfo->dobj.dump = owning_tab->dobj.dump;
> else
> seqinfo->dobj.dump |= owning_tab->dobj.dump;

Thank you for the new version of the patch. It looks good to me and it
works as expected after I tested it.

> The attached patch also gets rid of the dubious coding in
> getPublicationNamespaces. We might get push-back on that ignoring
> schemas belonging to extensions, but if so I'd prefer to see the
> behavior coded in a more transparent fashion.

I don't have a strong opinion about this part and I'm not sure that we
need to ignore mapping between a namespace and a publication if the
namespace belongs to the extension. Maybe we could always dump the
mapping if the publication itself is dumped.

But other than that it seems the patch breaks dumps of mapping of a
publication with the public namespace. The issue with the public
namespace is mentioned in the original thread of the feature of adding
schema mapping [1]. I couldn't find in the thread if it was addressed
in the end. With the new patch the following mapping won't be mapped:

create publication pub_test for tables in schema public;

pg_dump won't generate corresponding "ALTER PUBLICATION pub_test ADD
TABLES IN SCHEMA public".

1. https://www.postgresql.org/message-id/OS0PR01MB61137F0B8498E9241F2D960EFB149%40OS0PR01MB6113.jpnprd01.prod.outlook.com

--
Kind regards,
Artur