public inbox for [email protected]  
help / color / mirror / Atom feed
From: Peter Smith <[email protected]>
To: vignesh C <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Skipping schema changes in publication
Date: Thu, 19 May 2022 18:19:19 +1000
Message-ID: <CAHut+PsmfzDM-GCZL7DoTxYDcEnAaOY+KDEvH1bpzgNV_Suw9A@mail.gmail.com> (raw)
In-Reply-To: <CALDaNm0iZZDB300Dez_97S8G6_RW5QpQ8ef6X3wq8tyK-8wnXQ@mail.gmail.com>
References: <CALDaNm3=JrucjhiiwsYQw5-PGtBHFONa6F7hhWCXMsGvh=tamA@mail.gmail.com>
	<CALj2ACVOzhs+BD+abFV2x4oKJdsDNd6SgsE7r8UjnZDCKGEckA@mail.gmail.com>
	<CAA4eK1K6Kr88d2S0zFdHRMyuoaZeNh+ktU+oigmCuD09_x_-+g@mail.gmail.com>
	<CAHut+PsvC-NezO3MJkdyEz=G1QRje2LntjwhQiEeVbmhOQuBMA@mail.gmail.com>
	<CALDaNm18VH2j8cTqfELHQ=0ZNognbGBhbHPteJenWQC6C2dueQ@mail.gmail.com>
	<CALDaNm0k_0Ccj47wzJzzPFwgQB7w=R5+Q2_nSqYrmMmjhmcRUw@mail.gmail.com>
	<CAHut+Pv_0DwyWoGQNMF+G2AGqMuJTzWQKRtmxaC+=zLTPL-Zkw@mail.gmail.com>
	<CALDaNm2-GJt2HsYTkLqQ=ecm=R-vOBw1=aM_d2EiYbz39x_cTQ@mail.gmail.com>
	<TYCPR01MB8373C3120C2B3112001ED6F1EDCF9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
	<CALDaNm0iZZDB300Dez_97S8G6_RW5QpQ8ef6X3wq8tyK-8wnXQ@mail.gmail.com>

Below are my review comments for v6-0001.

======

1. General.

The patch failed 'publication' tests in the make check phase.

Please add this work to the commit-fest so that the 'cfbot' can report
such errors sooner.

~~~

2. src/backend/commands/publicationcmds.c - AlterPublicationReset

+/*
+ * Reset the publication.
+ *
+ * Reset the publication options, publication relations and
publication schemas.
+ */
+static void
+AlterPublicationReset(ParseState *pstate, AlterPublicationStmt *stmt,
+ Relation rel, HeapTuple tup)

SUGGESTION (Make the comment similar to the sgml text instead of
repeating "publication" 4x !)
/*
 * Reset the publication options, set the ALL TABLES flag to false, and
 * drop all relations and schemas that are associated with the publication.
 */

~~~

3. src/test/regress/expected/publication.out

make check failed. The diff is below:

@@ -1716,7 +1716,7 @@
 -- Verify that only superuser can reset a publication
 ALTER PUBLICATION testpub_reset OWNER TO regress_publication_user2;
 SET ROLE regress_publication_user2;
-ALTER PUBLICATION testpub_reset RESET; -- fail
+ALTER PUBLICATION testpub_reset RESET; -- fail - must be superuser
 ERROR:  must be superuser to RESET publication
 SET ROLE regress_publication_user;
 DROP PUBLICATION testpub_reset;


------
Kind Regards,
Peter Smith.
Fujitsu Australia





view thread (377+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Skipping schema changes in publication
  In-Reply-To: <CAHut+PsmfzDM-GCZL7DoTxYDcEnAaOY+KDEvH1bpzgNV_Suw9A@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox