public inbox for [email protected]
help / color / mirror / Atom feedFrom: vignesh C <[email protected]>
To: [email protected] <[email protected]>
Cc: 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: Wed, 18 May 2022 23:24:02 +0530
Message-ID: <CALDaNm3XiwZKFVkfQ+Q0GobWkyMb3ygKqghWQWuDj7ZxR9rUMw@mail.gmail.com> (raw)
In-Reply-To: <TYCPR01MB83737C28187A6E0BADAE98F0EDCF9@TYCPR01MB8373.jpnprd01.prod.outlook.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>
<TYCPR01MB83737C28187A6E0BADAE98F0EDCF9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
On Mon, May 16, 2022 at 2:00 PM [email protected]
<[email protected]> wrote:
>
> 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"
>
Modified
> (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.
Modified
> (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.
> "
>
Modified
> (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.
Changed it to allow except table only once
>
> (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 ?
This code can be removed because of grammar optimization, it will not
allow tables without "ALL TABLES". Removed this code
The v6 patch attached at [1] has the changes for the same.
[1] - https://www.postgresql.org/message-id/CALDaNm0iZZDB300Dez_97S8G6_RW5QpQ8ef6X3wq8tyK-8wnXQ%40mail.gma...
Regards,
Vignesh
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: <CALDaNm3XiwZKFVkfQ+Q0GobWkyMb3ygKqghWQWuDj7ZxR9rUMw@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