| From: | Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> |
|---|---|
| To: | vignesh C <vignesh21(at)gmail(dot)com> |
| Cc: | shveta malik <shveta(dot)malik(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(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-02 11:48:45 |
| Message-ID: | CANhcyEXP5BS+0Hq2o=Jmx6J5Nf+H0TW9yayO21NnVwtFqauDGw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, 29 Jan 2026 at 20:41, vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Wed, 28 Jan 2026 at 10:46, shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > Thank You for the patch.
> >
> > 1)
> > There are certain parts of Approach 3 still present in Approach 1, as
> > an example:
> >
> > 1a)
> > + For partitioned tables, only the root partitioned table may be specified
> > + in <literal>EXCEPT TABLE</literal>.
> >
> > 1b)
> > + /*
> > + * Only the topmost ancestor of a partitioned table can be specified
> > + * in EXCEPT TABLES clause of a FOR ALL TABLES publication. So fetch
> > + * the publications excluding the topmost ancestor only.
> > + */
> > + GetRelationPublications(llast_oid(ancestors), NULL, &exceptpuboids);
> > +
> >
> > 1c)
> > + /* Check if the partiton is part of EXCEPT list of any publication */
> > + GetRelationPublications(RelationGetRelid(attachrel), NULL, &except_pubids);
> > + if (except_pubids != NIL)
> > + ereport(ERROR,
> > + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> > + errmsg("cannot attach relation \"%s\" as partition because it is
> > part of EXCEPT list in publication",
> > + RelationGetRelationName(attachrel))));
> > +
> >
> > Overall, please take a diff of v35 and v37 to find such parts and
> > please correct these and others (if any).
> >
> > 2)
> > Also I don't think if below is correct statement for Approach 1:
> >
> > + * 2. For a partition, if the topmost ancestor is part of
> > + * the EXCEPT TABLE list, we don't publish it.
> >
> > Even if any ancestor is part of EXECPT list (not only top most) we
> > should not publish that partition, isn't it?
> >
> > 3)
> > I tried a scenario and found that incremental replication is not
> > working correctly. Attached the failing test as Approach1_v37_fail.txt
> >
> > Once these basic things are corrected, I can review further.
>
> These comments are addressed in the v38 version patch attached.
> Currently the approach-3 changes is present separately in
> v38-0002-Restrict-EXCEPT-TABLE-to-root-partitioned-tables-apporach-3.patch
> which can be applied on top of
> v38-0001-Skip-publishing-the-tables-specified-in-EXCEPT-T.patch.
> Similarly the approach-1 changes is present separately in
> v38-0002-handle-EXCEPT-TABLE-correctly-with-partitioned-approach-1.patch
> which can be applied on top of
> v38-0001-Skip-publishing-the-tables-specified-in-EXCEPT-T.patch.
> Currently few of the query are logged as LOG messages, I will reduce
> the log level for these queries once few rounds of review are
> completed on the queries.
Hi Vignesh,
I reviewed v38-0002-handle-EXCEPT-TABLE-correctly-with-partitioned-approach-1.patch
patch. Here are my comments:
1. in pgoutput.c:
@@ -2228,6 +2228,11 @@ get_rel_sync_entry(PGOutputData *data, Relation relation)
{
List *ancestors = get_partition_ancestors(relid);
+ GetRelationPublications(relid, NULL, &exceptpubids);
+
+ foreach_oid(ancestor, ancestors)
+ GetRelationPublications(ancestor, NULL, &exceptpubids);
+
pub_relid = llast_oid(ancestors);
ancestor_level = list_length(ancestors);
}
when we create a publication on a partitioned table and
publish_via_partition_root is false,
the changes of the partitions should not be published as discussed.
But, while testing I found that the changes are being published.
Test:
CREATE SCHEMA sc1;
CREATE TABLE sc1.t1(id int) PARTITION BY RANGE(id);
CREATE TABLE sc1.child1(id int) PARTITION BY RANGE(id);
CREATE TABLE sc1.child2 PARTITION OF sc1.t1 FOR VALUES FROM (101) TO (200);
CREATE TABLE sc1.child1_1 PARTITION OF sc1.child1 FOR VALUES FROM (0) TO (50);
CREATE TABLE sc1.child1_2 PARTITION OF sc1.child1 FOR VALUES FROM (51) TO (100);
ALTER TABLE sc1.t1 ATTACH PARTITION sc1.child1 FOR VALUES FROM (0) TO (100);
CREATE PUBLICATION pub1 FOR ALL TABLES EXCEPT TABLE (sc1.t1);
insert into sc1. t1 values (1), (60), (150);
I get following output:
postgres=# SELECT * FROM pg_logical_slot_get_binary_changes(
's1',
NULL,
NULL,
'proto_version', '1',
'publication_names', 'pub1'
);
lsn | xid | data
------------+-----+----------------------------------------------------------------------
0/0175A1A0 | 777 | \x42000000000175a3f80002eccfc016338000000309
0/0175A1A0 | 777 |
\x520000400a736331006368696c64315f31006400010069640000000017ffffffff
0/0175A1A0 | 777 | \x490000400a4e0001740000000131
0/0175A268 | 777 |
\x520000400d736331006368696c64315f32006400010069640000000017ffffffff
0/0175A268 | 777 | \x490000400d4e000174000000023630
0/0175A330 | 777 |
\x5200004007736331006368696c6432006400010069640000000017ffffffff
0/0175A330 | 777 | \x49000040074e00017400000003313530
0/0175A428 | 777 | \x4300000000000175a3f8000000000175a4280002eccfc0163380
(8 rows)
I think this check should be done irrespective of the value of
publish_via_partition_root.
+ foreach_oid(ancestor, ancestors)
+ GetRelationPublications(ancestor, NULL, &exceptpubids);
2. In get_rel_sync_entry we have a comment:
*
* 2. If pubviaroot is not set, check whether the relation itself
* is included in the EXCEPT TABLE list. If so, do not publish the
* change.
*
This comment does not seem correct. Even if publish_via_partition_root
is false, we should not publish changes if any of its ancestors is
excluded.
This is my understanding of approach 1 [1]. Correct me if I am wrong.
3. In tablesync.c:
+ /*
+ * Store the tables as a list of schemaname and tablename.
+ */
+ natt = 0;
this assignment 'natt = 0', is not required
4. While testing, I noticed that the new query introduced in tablesync
can be invoked for "FOR TABLE". SHould we only call it for "ALL
TABLES" publications?
+ if (server_version >= 190000 && !is_partition &&
+ lrel->relkind == RELKIND_PARTITIONED_TABLE)
+ {
+ resetStringInfo(&cmd);
+
+ /*
+ * This query recursively traverses the inheritance (partition) tree
+ * starting from the given table OID and determines which leaf
+ * relations should be included for replication. Exclusion propagates
+ * from parent to child, and a relation is also treated as excluded if
+ * it is explicitly marked with prexcept = true in pg_publication_rel
+ * for the specified publications. The final result returns only
+ * non excluded leaf relations.
+ */
Test:
Create publication for table sc1.t1 using (publish_via_partition_root
= true) and create subscription on it. In subscriber logs we can see
the logs for the new query.
sc1.t1 has the same structure as in comment 1.
5. I was testing some scenarios and found a difference in behaviour of
tablesync and incremental changes.
Consider the same table structure as comment1. But we have two publications as:
CREATE PUBLICATION pub1 FOR ALL TABLES EXCEPT TABLE (sc1.child1) WITH
(publish_via_partition_root = true);
CREATE PUBLICATION pub2 FOR ALL TABLES WITH (publish_via_partition_root = true);
And subscription is created
CREATE SUBSCRIPTION sub1 CONNECTION 'host=localhost port=5432
dbname=postgres' PUBLICATION pub1, pub2;
Before creating the subscription I did a insert on publisher node:
INSERT INTO sc1.t1 VALUES(1), (51), (101);
Output after tablesync:
postgres=# select * from sc1.t1;
id
-----
101
(1 row)
Now, I again did a insert on publisher node:
INSERT INTO sc1.t1 VALUES(1), (51), (101);
Output after incremental sync:
postgres=# select * from sc1.t1;
id
-----
101
1
51
101
(4 rows)
What should be the behaviour if a partitioned table is published by a
publication and excluded by another publication?
I checked the behaviour of publication with row_filter for similar conditions:
CREATE PUBLICATION pub1 FOR TABLE sc1.t1 WHERE (id > 100) WITH
(publish_via_partition_root = true);
CREATE PUBLICATION pub2 FOR TABLE sc1.t1 WITH
(publish_via_partition_root = true);
Here is the behaviour:
Before creating the publication I did a insert on publisher node:
INSERT INTO sc1.t1 VALUES(1), (51), (101);
Output after tablesync:
postgres=# select * from sc1.t1;
id
-----
101
1
51
(3 rows)
Now, I again did a insert on publisher node:
INSERT INTO sc1.t1 VALUES(1), (51), (101);
Output after incremental sync:
postgres=# select * from sc1.t1;
id
-----
101
1
51
1
51
101
(6 rows)
So, I think in the EXCEPT table case we should follow the same
behaviour with tablesync i.e. publish the change if any of the
publications publishes it . Thoughts?
Thanks,
Shlok Kyal
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Álvaro Herrera | 2026-02-02 11:50:50 | Re: splitting pg_resetwal output strings |
| Previous Message | John Naylor | 2026-02-02 11:30:13 | Re: Undefined behavior detected by new clang's ubsan |