Horton

Re: Skipping schema changes in publication

vignesh C <vignesh21@gmail.com>

View thread
From: vignesh C <vignesh21@gmail.com>
To: "osumi.takamichi@fujitsu.com" <osumi.takamichi@fujitsu.com>
Cc: 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-05-18T17:54:02Z
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 Mon, May 16, 2022 at 2:00 PM osumi.takamichi@fujitsu.com
<osumi.takamichi@fujitsu.com> wrote:
>
> On Saturday, May 14, 2022 10:33 PM vignesh C <vignesh21@gmail.com> 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.gmail.com

Regards,
Vignesh