Re: Skipping schema changes in publication

On Tue, 17 Mar 2026 at 16:31, shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Tue, Mar 17, 2026 at 12:23 PM vignesh C <vignesh21(at)gmail(dot)com> 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.
> */

These comments are addressed in the v65 version patch attached. Also
the comments from [1] have been addressed in this.
[1] - https://www.postgresql.org/message-id/CAA4eK1%2BmSpCzj%2BB2PW_68DJpXHA0KMgT9Nrz9P83_c1vdKya8g%40mail.gmail.com

Regards,
Vignesh