public inbox for [email protected]  
help / color / mirror / Atom feed
From: [email protected] <[email protected]>
To: 'vignesh C' <[email protected]>
To: Amit Kapila <[email protected]>
Cc: Euler Taveira <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: RE: Skipping schema changes in publication
Date: Tue, 26 Apr 2022 06:02:46 +0000
Message-ID: <TYCPR01MB8373FE72A4CDBE7A5E8886EBEDFB9@TYCPR01MB8373.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CALDaNm2KJRmQMcpOn0QPoKm6ET93Z-PSmbwwdm8O0Lr3eWqkTw@mail.gmail.com>
References: <CALDaNm3=JrucjhiiwsYQw5-PGtBHFONa6F7hhWCXMsGvh=tamA@mail.gmail.com>
	<CALDaNm1G0=0J2O+6pMStKuWUjxJJT5vbO-5jiUqzYVZM_sJ2+w@mail.gmail.com>
	<CALDaNm3Rc2KxGzLy-4qxFS+MWS6GN-ROVoPEb1MFA53W3iAqog@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CAA4eK1KikfJMVggZ_aNWPio7zrdObVdidG3SZVWEZ_LOU=vXkg@mail.gmail.com>
	<CALDaNm2KJRmQMcpOn0QPoKm6ET93Z-PSmbwwdm8O0Lr3eWqkTw@mail.gmail.com>

On Thursday, April 21, 2022 12:15 PM vignesh C <[email protected]> wrote:
> Updated patch by changing the syntax to use EXCEPT instead of SKIP.
Hi


This is my review comments on the v2 patch.

(1) gram.y

I think we can make a unified function that merges
preprocess_alltables_pubobj_list with check_except_in_pubobj_list.

With regard to preprocess_alltables_pubobj_list,
we don't use the 2nd argument "location" in this function.

(2) create_publication.sgml

+  <para>
+   Create a publication that publishes all changes in all the tables except for
+   the changes of <structname>users</structname> and
+   <structname>departments</structname> table;

This sentence should end ":" not ";".

(3) publication.out & publication.sql

+-- fail - can't set except table to schema  publication
+ALTER PUBLICATION testpub_forschema SET EXCEPT TABLE testpub_tbl1;

There is one unnecessary space in the comment.
Kindly change from "schema  publication" to "schema publication".

(4) pg_dump.c & describe.c

In your first email of this thread, you explained this feature
is for PG16. Don't we need additional branch for PG16 ?

@@ -6322,6 +6328,21 @@ describePublications(const char *pattern)
                        }
                }

+               if (pset.sversion >= 150000)
+               {


@@ -4162,7 +4164,7 @@ getPublicationTables(Archive *fout, TableInfo tblinfo[], int numTables)
        /* Collect all publication membership info. */
        if (fout->remoteVersion >= 150000)
                appendPQExpBufferStr(query,
-                                                        "SELECT tableoid, oid, prpubid, prrelid, "
+                                                        "SELECT tableoid, oid, prpubid, prrelid, prexcept,"


(5) psql-ref.sgml

+        If <literal>+</literal> is appended to the command name, the tables,
+        except tables and schemas associated with each publication are shown as
+        well.

I'm not sure if "except tables" is a good description.
I suggest "excluded tables". This applies to the entire patch,
in case if this is reasonable suggestion.


Best Regards,
	Takamichi Osumi



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: <TYCPR01MB8373FE72A4CDBE7A5E8886EBEDFB9@TYCPR01MB8373.jpnprd01.prod.outlook.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