RE: Added schema level support for publication. - Mailing list pgsql-hackers

Wednesday, July 14, 2021 6:17 PM vignesh C <vignesh21@gmail.com> wrote:
> On Tue, Jul 13, 2021 at 12:06 PM tanghy.fnst@fujitsu.com
> <tanghy.fnst@fujitsu.com> wrote:
> >
> > On Monday, July 12, 2021 5:36 PM vignesh C <vignesh21@gmail.com> wrote:
> > >
> > > Thanks for reporting this issue, this issue is fixed in the v10
> > > patch attached at [1].
> > > [1] - https://www.postgresql.org/message-id/CALDaNm2%2BtR%2B8R-
> > > sD1CSyMbZcZbkintZE-avefjsp7LCkm6HMmw%40mail.gmail.com
> >
> > Thanks for fixing it.
> >
> > By applying your V10 patch, I saw three problems, please have a look.
> >
> > 1. An issue about pg_dump.
> > When public schema was published, the publication was created in the
> > output file, but public schema was not added to it. (Other schemas
> > could be added as expected.)
> >
> > I looked into it and found that selectDumpableNamespace function marks
> DUMP_COMPONENT_DEFINITION as needless when the schema is public,
> leading to schema public is ignored in getPublicationSchemas. So we'd better
> check whether schemas should be dumped in another way.
> >
> > I tried to fix it with the following change, please have a look.
> > (Maybe we also need to add some comments for it.)
> >
> > diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
> > index f6b4f12648..a327d2568b 100644
> > --- a/src/bin/pg_dump/pg_dump.c
> > +++ b/src/bin/pg_dump/pg_dump.c
> > @@ -4206,7 +4206,8 @@ getPublicationSchemas(Archive *fout,
> NamespaceInfo nspinfo[], int numSchemas)
> >                  * Ignore publication membership of schemas whose
> definitions are not
> >                  * to be dumped.
> >                  */
> > -               if (!(nsinfo->dobj.dump &
> DUMP_COMPONENT_DEFINITION))
> > +               if (!((nsinfo->dobj.dump &
> DUMP_COMPONENT_DEFINITION)
> > +                       || (strcmp(nsinfo->dobj.name, "public") == 0
> > + && nsinfo->dobj.dump != DUMP_COMPONENT_NONE)))
> >                         continue;
> >
> >                 pg_log_info("reading publication membership for schema
> > \"%s\"",
> 
> I felt it is intentionally done like that as the pubic schema is created by default,
> hence it is not required to dump else we will get errors while restoring.
> Thougths?

Thanks for the new patches and I also looked at this issue.

For user defined schema and publication:
--------------------------
create schema s1;
create publication pub2 for SCHEMA s1;
--------------------------

pg_dump will only generate the following SQLs:

------pg_dump result------
CREATE PUBLICATION pub2 WITH (publish = 'insert, update, delete, truncate');
ALTER PUBLICATION pub2 ADD SCHEMA s1;
--------------------------

But for the public schema:
--------------------------
create publication pub for SCHEMA public;
--------------------------

pg_dump will only generate the following SQL:

------pg_dump result------
CREATE PUBLICATION pub WITH (publish = 'insert, update, delete, truncate');
--------------------------

It didn't generate SQL like "ALTER PUBLICATION pub ADD SCHEMA public;" which
means the public schema won't be published after restoring. So, I think we'd
better let the pg_dump generate the ADD SCHEMA public SQL. Thoughts ?


Best regards,
Hou zhijie