On Mon, Mar 2, 2026 at 6:51 PM Shlok Kyal <[email protected]> wrote:
>
> Attached the updated v54 patch.
>

Few comments for v54 patch -
1) The partition description shows the publication name which it is
excluded from.
simple test case:

 CREATE TABLE t_part (a int) PARTITION BY RANGE(a);
 CREATE TABLE t_part_p1 PARTITION OF t_part FOR VALUES FROM (0) TO (100);
 CREATE PUBLICATION pub_p1 FOR ALL TABLES EXCEPT TABLE (t_part);
postgres=# \d t_part_p1
             Table "public.t_part_p1"
 Column |  Type   | Collation | Nullable | Default
--------+---------+-----------+----------+---------
 a      | integer |           |          |
Partition of: t_part FOR VALUES FROM (0) TO (100)
Publications:
    "pub_p1"
~~~

2) File: 037_rep_changes_except_table.pl:147
+# Verify that data inserted into a table listed in the EXCEPT clause is not
+# published.
+$result = $node_publisher->safe_psql('postgres',
+ "SELECT count(*) = 0 FROM
pg_logical_slot_get_binary_changes('test_slot', NULL, NULL,
'proto_version', '1', 'publication_names', 'tap_sub_schema')"
+);

There seems to be a mistake in the publication name in the above call
as tap_sub_schema is a subscription name.
~~~
Couple of minor comments:
3) File: pgouput.c
+ /*
+ * For a parition, changes are published via top-most
+ * ancestor when pubviaroot is true, so populate pub_relid
+ * accordingly
+ */

3a) typo parition -> partition
3b) There should be a full stop (.) at the end i.e. after accordingly
~~~

File: pg_publication.c
 + else
 + errormsg = gettext_noop("cannot add relation \"%s\" to publication");
 +
 +
 + /* If in EXCEPT clause, must be root partitioned table */

>> there is an extra empty line
~~~

--
Thanks,
Nisha


Reply via email to