Horton

Re: Skipping schema changes in publication

vignesh C <vignesh21@gmail.com>

View thread
From: vignesh C <vignesh21@gmail.com>
To: Nitin Jadhav <nitinjadhavpostgres@gmail.com>
Cc: "osumi.takamichi@fujitsu.com" <osumi.takamichi@fujitsu.com>, Peter Smith <smithpb2250@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-08-18T14:26:54Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix miscellaneous issues in EXCEPT publication clause.

  2. Change syntax of EXCEPT TABLE clause in publication commands.

  3. Add support for EXCEPT TABLE in ALTER PUBLICATION.

  4. Allow table exclusions in publications via EXCEPT TABLE.

  5. Add wait_for_subscription_sync for TAP tests.

On Thu, Aug 18, 2022 at 12:33 PM Nitin Jadhav
<nitinjadhavpostgres@gmail.com> wrote:
>
> I spent some time on understanding the proposal and the patch. Here
> are a few comments wrt the test cases.
>
> > +ALTER PUBLICATION testpub_reset ADD TABLE pub_sch1.tbl1;
> > +
> > +-- Verify that tables associated with the publication are dropped after RESET
> > +\dRp+ testpub_reset
> > +ALTER PUBLICATION testpub_reset RESET;
> > +\dRp+ testpub_reset
> >
> > +ALTER PUBLICATION testpub_reset ADD ALL TABLES IN SCHEMA public;
> > +
> > +-- Verify that schemas associated with the publication are dropped after RESET
> > +\dRp+ testpub_reset
> > +ALTER PUBLICATION testpub_reset RESET;
> > +\dRp+ testpub_reset
>
> The results for the above two cases are the same before and after the
> reset. Is there any way to verify that?

If you see the expected, first \dRp+ command includes:
+Tables:
+    "pub_sch1.tbl1"
The second \dRp+ does not include the Tables.
We are trying to verify that after reset, the tables will be removed
from the publication.
The second test is similar to the first, the only difference here is
that we test schema instead of tables. i.e we verify that the schemas
will be removed from the publication.

> ---
>
> > +-- Can't add EXCEPT TABLE to 'FOR ALL TABLES' publication
> > +ALTER PUBLICATION testpub_reset ADD ALL TABLES EXCEPT TABLE pub_sch1.tbl1;
> > +
> >
> > +-- Can't add EXCEPT TABLE to 'FOR TABLE' publication
> > +ALTER PUBLICATION testpub_reset ADD ALL TABLES EXCEPT TABLE pub_sch1.tbl1;
> > +
> >
> > +-- Can't add EXCEPT TABLE to 'FOR ALL TABLES IN SCHEMA' publication
> > +ALTER PUBLICATION testpub_reset ADD ALL TABLES EXCEPT TABLE pub_sch1.tbl1;
> > +
>
> I did not understand the objective of these tests. I think we need to
> improve the comments.

There are different publications like "ALL TABLES", "TABLE", "ALL
TABLES IN SCHEMA" publications. Here we are trying to verify that
except tables cannot be added to "ALL TABLES", "TABLE", "ALL TABLES IN
SCHEMA" publications.
If you see the expected file, you will see the following error:
+-- Can't add EXCEPT TABLE to 'FOR ALL TABLES' publication
+ALTER PUBLICATION testpub_reset ADD ALL TABLES EXCEPT TABLE pub_sch1.tbl1;
+ERROR:  adding ALL TABLES requires the publication to have default
publication parameter values
+DETAIL:  ALL TABLES flag should not be set and no tables/schemas
should be associated.
+HINT:  Use ALTER PUBLICATION ... RESET to reset the publication

I felt the existing comment is ok. Let me know if you still feel any
change is required.

Regards,
Vignesh