On Mon, Feb 23, 2026 at 4:46 PM Amit Kapila <[email protected]> wrote:
>
> On Mon, Feb 23, 2026 at 11:37 AM Shlok Kyal <[email protected]> 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
~~
Reviewing further.
thanks
Shveta
