public inbox for [email protected]
help / color / mirror / Atom feedFrom: shveta malik <[email protected]>
To: vignesh C <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: Shlok Kyal <[email protected]>
Cc: Nisha Moond <[email protected]>
Cc: Ashutosh Sharma <[email protected]>
Cc: David G. Johnston <[email protected]>
Cc: Dilip Kumar <[email protected]>
Cc: Zhijie Hou (Fujitsu) <[email protected]>
Cc: YeXiu <[email protected]>
Cc: Ian Lawrence Barwick <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: shveta malik <[email protected]>
Subject: Re: Skipping schema changes in publication
Date: Tue, 17 Mar 2026 16:31:43 +0530
Message-ID: <CAJpy0uCN4gfP7fSt__KdW5wYQ82650Z6L4YLnjRHZTQ1yir1mg@mail.gmail.com> (raw)
In-Reply-To: <CALDaNm2OOgmNOPpABUU+AXzHhfrLG9HMfSd3jfNe=t3dc-kp1Q@mail.gmail.com>
References: <CAJpy0uB20MhJJEaPJdm31t4fykJ+fChA_76jU2P9HX5knbJvAA@mail.gmail.com>
<CAD21AoCC8XuwfX62qKBSfHUAoww_XB3_84HjswgL9jxQy696yw@mail.gmail.com>
<OS9PR01MB12149EA0C749BC29C7C949E32F544A@OS9PR01MB12149.jpnprd01.prod.outlook.com>
<CAD21AoBbZEshyaK0PeiF_J4_S75EfF=Gcs=C+X-osoVoUnawuQ@mail.gmail.com>
<CAHut+PssG+sHeV+Xo0g=S7xBb9FgDPjHYDR4iSuOdYXDq-Psng@mail.gmail.com>
<CAA4eK1LaSfAG7UAuy1xpnkWKM_YtrPuhbgAxYBFY3Sp_v_KqoQ@mail.gmail.com>
<CAD21AoAb8E8krN63cY_U7RQs9v-zkqUZyKT_UVKDwKfExtvTBg@mail.gmail.com>
<CAA4eK1K1GLR7DXSABayQE+pWM=v1ODD6haPYxuDhAYwJN5gjzg@mail.gmail.com>
<CALDaNm2kvFahDDvdgCNo=Nv-COz_N5Xw8YmzQBN2bd3g=N81fQ@mail.gmail.com>
<CAHut+PsCqTR_kQu5M1TqBjnE6KM5cO22aH8boHfpMa_gSJBmWg@mail.gmail.com>
<CALDaNm2OOgmNOPpABUU+AXzHhfrLG9HMfSd3jfNe=t3dc-kp1Q@mail.gmail.com>
On Tue, Mar 17, 2026 at 12:23 PM vignesh C <[email protected]> wrote:
>
> Thanks for the comments, the agreed comments have been addressed in
> the v64 version patch attached.
>
Please find a few comments:
1)
+ The
+ <literal>SET ALL TABLES</literal> clause is used to update the
+ <literal>EXCEPT TABLE</literal> list of a <literal>FOR ALL TABLES</literal>
+ publication. If <literal>EXCEPT TABLE</literal> is specified with a list of
+ tables, the existing except table list is replaced with the
specified tables.
+ If <literal>EXCEPT TABLE</literal> is omitted, the existing except table
+ list is cleared.
How about changing it to (or anything better to reflect new changes):
The SET ALL TABLES clause can transform an empty publication, or one
defined for ALL SEQUENCES (or both ALL TABLES and ALL SEQUENCES), into
a publication defined for ALL TABLES. Likewise, SET ALL SEQUENCES can
convert an empty publication, or one defined for ALL TABLES (or both
ALL TABLES and ALL SEQUENCES), into a publication defined for ALL
SEQUENCES.
In addition, SET ALL TABLES may be used to update the EXCEPT TABLE
list of a FOR ALL TABLES publication. If EXCEPT TABLE is specified
with a list of tables, the existing exclusion list is replaced with
the specified tables. If EXCEPT TABLE is omitted, the existing
exclusion list is cleared.
2)
+bool
+is_include_relation_publication(Oid pubid)
The name 'is_include_relation_publication' looks slightly odd to me.
Few options are: is_explicit_table_publication,
is_table_list_publication, is_table_publication. Or anything better if
you can think of?
3)
is_include_relation_publication:
+ /* If we find even one included relation, we are done */
+ if (!pubrel->prexcept)
+ {
+ result = true;
+ break;
+ }
we can break the loop irrespective of the 'prexcept' flag as we can
never have a combination of mixed prexcept entries for the same pub.
Whether all will be with prexcept=true or all will be false. Even a
loop is not needed. We can fetch the first entry alone (similar to
is_schema_publication) and if that is valid, we can check the flag and
return accordingly. Something like:
if (HeapTupleIsValid())
{
pubrel = (Form_pg_publication_rel) GETSTRUCT(tup);
result = !pubrel->prexcept
}
4)
publication_add_relation:
+ /*
+ * True if EXCEPT tables require explicit relcache invalidation. If
+ * 'puballtables' changes, global invalidation covers them.
+ */
+ inval_except_table = (stmt != NULL) &&
+ (stmt->for_all_tables == pub->alltables);
It took me some time to figure out why we don't need invalidation for
the case where we are converting ALL SEQ to ALL TABLEs EXCEPT(..). I
think it is worth adding more comments here. Suggestion:
/*
* Determine whether EXCEPT tables require explicit relcache invalidation.
*
* For CREATE PUBLICATION with EXCEPT tables, invalidation is not needed,
* since it is handled when marking the publication as ALL TABLES.
*
* For ALTER PUBLICATION, invalidation is needed only when adding an EXCEPT
* table to a publication already marked as ALL TABLES. For publications
* that were originally empty or defined as ALL SEQUENCES and are being
* converted to ALL TABLES, invalidation is skipped here, as it is handled
* when marking the publication as ALL TABLES.
*/
thanks
Shveta
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], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Skipping schema changes in publication
In-Reply-To: <CAJpy0uCN4gfP7fSt__KdW5wYQ82650Z6L4YLnjRHZTQ1yir1mg@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