From: | "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com> |
---|---|
To: | vignesh C <vignesh21(at)gmail(dot)com>, "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com> |
Cc: | Ajin Cherian <itsajin(at)gmail(dot)com>, Rahila Syed <rahilasyed90(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
Subject: | RE: Added schema level support for publication. |
Date: | 2021-07-19 04:02:04 |
Message-ID: | OS0PR01MB611393384AB6280030A24FC6FBE19@OS0PR01MB6113.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Friday, July 16, 2021 6:10 PM vignesh C <vignesh21(at)gmail(dot)com>
> On Wed, Jul 14, 2021 at 6:25 PM houzj(dot)fnst(at)fujitsu(dot)com
> <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> > Wednesday, July 14, 2021 6:17 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > > On Tue, Jul 13, 2021 at 12:06 PM tanghy(dot)fnst(at)fujitsu(dot)com
> > > <tanghy(dot)fnst(at)fujitsu(dot)com> wrote:
> > > >
> > > > On Monday, July 12, 2021 5:36 PM vignesh C <vignesh21(at)gmail(dot)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 ?
>
> Thanks for reporting this issue, this issue is fixed in the v12 patch attached.
>
I tested your v12 patch and found a problem in the following case.
Step 1:
postgres=# create schema s1;
CREATE SCHEMA
postgres=# create table s1.t1 (a int);
CREATE TABLE
postgres=# create publication pub_t for table s1.t1;
CREATE PUBLICATION
postgres=# create publication pub_s for schema s1;
CREATE PUBLICATION
Step 2:
pg_dump -N s1
I dumped and excluded schema s1, pg_dump generated the following SQL:
-------------------------------
ALTER PUBLICATION pub_s ADD SCHEMA s1;
I think it was not expected because SQL like "ALTER PUBLICATION pub_t ADD TABLE s1.t1" was not generated in my case. Thoughts?
Regards
Tang
From | Date | Subject | |
---|---|---|---|
Next Message | Fujii Masao | 2021-07-19 04:30:59 | Re: Doc necessity for superuser privileges to execute pg_import_system_collations() |
Previous Message | Peter Smith | 2021-07-19 03:49:49 | Re: [HACKERS] logical decoding of two-phase transactions |