| From: | Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> |
|---|---|
| To: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, YeXiu <1518981153(at)qq(dot)com>, Ian Lawrence Barwick <barwick(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Skipping schema changes in publication |
| Date: | 2026-02-25 17:13:10 |
| Message-ID: | CANhcyEUur4fH9cB0NgiBHcV7Q0aoL+cC8qB2DCAJVcsdOJ+nqg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, 25 Feb 2026 at 16:01, shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Mon, Feb 23, 2026 at 4:46 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Mon, Feb 23, 2026 at 11:37 AM Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> wrote:
> > >
> > > I have also modified the error message as suggested by Shveta in [2].
> > > Attached the latest v48 patch.
> > >
> >
> > I see that the second patch (0002) brings complexity in the patch to
> > deal with following points: (a) The first complexity is if one of the
> > partitions is specified then how to compute the initial set of
> > relations to copy when pubviaroot is true. This is complex because we
> > need to exclude the partitions specified. (b) The other complexity is
> > combining Except list containing partitions and other publications
> > specifying partitions or partitioned tables both during replication
> > and probably during initial sync.
> >
> > I think it will be better if for the first version, we allow only root
> > partitioned table to be specified in the Except Table list. This would
> > mean that if the user tries to attach that root partition table to
> > another root then we should give an error. If we go via this route, it
> > will be important to allow users to remove some tables from the Except
> > list, so we can provide Alter Publication <pub_name> Set Except Table
> > (table_names).
> >
> > I think excluding specific partitions may also have some use cases but
> > adding additional complexity and maintenance effort is worth, if there
> > is a real field demand for such a feature.
> >
> > Thoughts?
> >
>
> I fully agree. The additional complexity does not appear to be worth
> the value right now, unless we receive meaningful user feedback that
> justifies implementing it.
>
> Thanks Vignesh for the patch supporting this approach. Please find a
> few comments on v49-001:
>
> 1)
> + in <literal>EXCEPT TABLE</literal>. Doing so excludes the root table and
> + all of its partitions from replication, regardless of the value of
> + <literal>publish_via_partition_root</literal>. The optional
> + <literal>*</literal> has no effect for partitioned tables.
> + </para>
>
> a)
> I feel it is not needed to mention 'regardless of
> publish_via_partition_root' as generally that never decides what is
> included and what is not. Same is the case here.
>
> b)
> We shall mention both ONLY and *. Infact it is 'ONLY' which has no
> effect. * is the default in this case. But still to make it a more
> generic statement, we shall mention both for partitioned tables.
>
> 2)
> + <para>
> + Create a publication that publishes all sequences for synchronization, and
> + all changes in all tables except <structname>users</structname> and
> + <structname>departments</structname>:
> +<programlisting>
> +CREATE PUBLICATION all_sequences_tables_except FOR ALL SEQUENCES, ALL
> TABLES EXCEPT (users, departments);
>
> If I run the command given in example, it asserts.
>
> TRAP: failed Assert("!stmt->for_all_sequences"), File:
> "publicationcmds.c", Line: 942, PID: 70620
> postgres: shveta postgres [local] CREATE
> PUBLICATION(ExceptionalCondition+0xbb)[0x611280ed8c48]
>
> 3)
> -check_publication_add_relation(Relation targetrel)
> +check_publication_add_relation(Relation targetrel, PublicationRelInfo *pri)
>
> We can avoid passing 'targetrel' now. PublicationRelInfo has that info inside.
>
> 4)
> postgres=# create publication pub2 for all tables except (tab_part_1);
> ERROR: partition "tab_part_1" cannot be excluded using EXCEPT TABLE
>
> Other errors are in active voice (eg: cannot use system column \"%s\"
> in publication column list, cannot use publication EXCEPT clause for
> relation \"%s\).
>
> Shall we also keep this the same way? I think we can mimic the error
> for tmp and unlogged tables which is:
>
> postgres=# create publication pub2 for all tables except (tmp1);
> ERROR: cannot use publication EXCEPT clause for relation "tmp1"
> DETAIL: This operation is not supported for temporary tables.
>
> We can convert concerned one to:
>
> postgres=# create publication pub2 for all tables except (tab_part_1);
> ERROR: cannot use publication EXCEPT clause for partition "tab_part_1"
> DETAIL: This operation is not supported for individual partitions.
>
>
> 5)
> + if (pub->alltables && pri->except && targetrel->rd_rel->relispartition)
> + ereport(ERROR,
> + errmsg("partition \"%s\" cannot be excluded using EXCEPT TABLE",
> + RelationGetRelationName(targetrel)));
> +
> + check_publication_add_relation(targetrel, pri);
>
> I feel we can push the partition check and error to
> check_publication_add_relation() itself. If we do so, we can retain
> the exactly same errormsg for parition case as well which is:
> errormsg = gettext_noop("cannot use publication EXCEPT clause for
> relation \"%s\"");
>
> And we can just change DETAIL as suggested in pt 4.
>
> 6)
> +++ b/src/backend/catalog/pg_subscription.c
> -/*
> - * Add a comma-separated list of publication names to the 'dest' string.
> - */
> -void
> -GetPublicationsStr(List *publications, StringInfo dest, bool quote_literal)
> -{
>
> Do we still need to move GetPublicationsStr() to logical.c? I see the
> only extra call is now in pgoutput.c which already includes
> catalog/pg_subscription.h
> ~~
Hi Shveta, Ashutosh, Chao-san,
Thanks for reviewing the patch.
I have addressed the above comments and the comments in [1] and [2].
Attached is the latest v50 version.
I have also split the patch 0002 to 0002 and 0003
0001 - Support CREATE PUBLICATION ... EXCEPT TABLE syntax
0002 - Support ALTER PUBLICATION ... SET EXCEPT TABLE .. syntax
0003 - Support ALTER PUBLICATION ... DROP EXCEPT TABLE .. syntax
[1]: https://www.postgresql.org/message-id/CAE9k0P=wzzOXykzgj-7Dw7E9r8k0PX633RY3TEVDpwLue004tg@mail.gmail.com
[2]: https://www.postgresql.org/message-id/0DDF38E7-3033-47E1-98C1-49A1378526B3@gmail.com
Thanks,
Shlok Kyal
| Attachment | Content-Type | Size |
|---|---|---|
| v50-0001-Skip-publishing-the-tables-specified-in-EXCEPT-T.patch | application/octet-stream | 83.1 KB |
| v50-0003-Support-DROP-EXCEPT-TABLE-in-ALTER-PUBLICATION.patch | application/octet-stream | 10.4 KB |
| v50-0002-Support-SET-EXCEPT-TABLE-in-ALTER-PUBLICATION.patch | application/octet-stream | 23.0 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Zsolt Parragi | 2026-02-25 17:16:33 | Re: [oauth] Stabilize the libpq-oauth ABI (and allow alternative implementations?) |
| Previous Message | Peter Eisentraut | 2026-02-25 16:40:10 | Re: Optimize SELECT * in EXISTS |