public inbox for [email protected]
help / color / mirror / Atom feedFrom: [email protected] <[email protected]>
To: 'vignesh C' <[email protected]>
To: Peter Smith <[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: Mon, 16 May 2022 08:29:58 +0000
Message-ID: <TYCPR01MB83737C28187A6E0BADAE98F0EDCF9@TYCPR01MB8373.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CALDaNm2-GJt2HsYTkLqQ=ecm=R-vOBw1=aM_d2EiYbz39x_cTQ@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>
On Saturday, May 14, 2022 10:33 PM vignesh C <[email protected]> wrote:
> Thanks for the comments, the attached v5 patch has the changes for the same.
> Also I have made the changes for SKIP Table based on the new syntax, the
> changes for the same are available in
> v5-0002-Skip-publishing-the-tables-specified-in-EXCEPT-TA.patch.
Hi,
Several comments on v5-0002.
(1) One unnecessary space before "except_pub_obj_list" syntax definition
+ except_pub_obj_list: ExceptPublicationObjSpec
+ { $$ = list_make1($1); }
+ | except_pub_obj_list ',' ExceptPublicationObjSpec
+ { $$ = lappend($1, $3); }
+ | /*EMPTY*/ { $$ = NULL; }
+ ;
+
From above part, kindly change
FROM:
" except_pub_obj_list: ExceptPublicationObjSpec"
TO:
"except_pub_obj_list: ExceptPublicationObjSpec"
(2) doc/src/sgml/ref/create_publication.sgml
(2-1)
@@ -22,7 +22,7 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
CREATE PUBLICATION <replaceable class="parameter">name</replaceable>
- [ FOR ALL TABLES
+ [ FOR ALL TABLES [EXCEPT TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ] [, ... ]]
| FOR <replaceable class="parameter">publication_object</replaceable> [, ... ] ]
[ WITH ( <replaceable class="parameter">publication_parameter</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] ) ]
Here I think we need to add two more whitespaces around square brackets.
Please change
FROM:
"[ FOR ALL TABLES [EXCEPT TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ] [, ... ]]"
TO:
"[ FOR ALL TABLES [ EXCEPT TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ] [, ... ] ]"
When I check other documentations, I see whitespaces before/after square brackets.
(2-2)
This whitespace alignment applies to alter_publication.sgml as well.
(3)
@@ -156,6 +156,24 @@ CREATE PUBLICATION <replaceable class="parameter">name</replaceable>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><literal>EXCEPT TABLE</literal></term>
+ <listitem>
+ <para>
+ Marks the publication as one that excludes replicating changes for the
+ specified tables.
+ </para>
+
+ <para>
+ <literal>EXCEPT TABLE</literal> can be specified only for
+ <literal>FOR ALL TABLES</literal> publication. It is not supported for
+ <literal>FOR ALL TABLES IN SCHEMA </literal> publication and
+ <literal>FOR TABLE</literal> publication.
+ </para>
+ </listitem>
+ </varlistentry>
+
This EXCEPT TABLE clause is only for FOR ALL TABLES.
So, how about extracting the main message from above part and
moving it to an exising paragraph below, instead of having one independent paragraph ?
<varlistentry>
<term><literal>FOR ALL TABLES</literal></term>
<listitem>
<para>
Marks the publication as one that replicates changes for all tables in
the database, including tables created in the future.
</para>
</listitem>
</varlistentry>
Something like
"Marks the publication as one that replicates changes for all tables in
the database, including tables created in the future. EXCEPT TABLE indicates
excluded tables for the defined publication.
"
(4) One minor confirmation about the syntax
Currently, we allow one way of writing to indicate excluded tables like below.
(example) CREATE PUBLICATION mypub FOR ALL TABLES EXCEPT TABLE tab3, tab4, EXCEPT TABLE tab5;
This is because we define ExceptPublicationObjSpec with EXCEPT TABLE.
Is it OK to have a room to write duplicate "EXCEPT TABLE" clauses ?
I think there is no harm in having this,
but I'd like to confirm whether this syntax might be better to be adjusted or not.
(5) CheckAlterPublication
+
+ if (excepttable && !stmt->for_all_tables)
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("publication \"%s\" is not defined as FOR ALL TABLES",
+ NameStr(pubform->pubname)),
+ errdetail("except table cannot be added to, dropped from, or set on NON ALL TABLES publications.")));
Could you please add a test for this ?
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: <TYCPR01MB83737C28187A6E0BADAE98F0EDCF9@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