public inbox for [email protected]  
help / color / mirror / Atom feed
From: Shlok Kyal <[email protected]>
To: Peter Smith <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Zhijie Hou (Fujitsu) <[email protected]>
Cc: YeXiu <[email protected]>
Cc: Ian Lawrence Barwick <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Skipping schema changes in publication
Date: Tue, 9 Dec 2025 23:19:26 +0530
Message-ID: <CANhcyEXT=DPeULFL2udhcyO1340Y1fGqgpmUqpiCh++JnC8caw@mail.gmail.com> (raw)
In-Reply-To: <CAHut+PsqV+dkPyYZ2BxWKBq7+TkrjCwBHBAQ3bHOSEChovD+ZA@mail.gmail.com>
References: <CALDaNm3=JrucjhiiwsYQw5-PGtBHFONa6F7hhWCXMsGvh=tamA@mail.gmail.com>
	<CALj2ACVOzhs+BD+abFV2x4oKJdsDNd6SgsE7r8UjnZDCKGEckA@mail.gmail.com>
	<CAA4eK1K6Kr88d2S0zFdHRMyuoaZeNh+ktU+oigmCuD09_x_-+g@mail.gmail.com>
	<CAHut+PsvC-NezO3MJkdyEz=G1QRje2LntjwhQiEeVbmhOQuBMA@mail.gmail.com>
	<CALDaNm18VH2j8cTqfELHQ=0ZNognbGBhbHPteJenWQC6C2dueQ@mail.gmail.com>
	<CALDaNm0k_0Ccj47wzJzzPFwgQB7w=R5+Q2_nSqYrmMmjhmcRUw@mail.gmail.com>
	<CAHut+Pv_0DwyWoGQNMF+G2AGqMuJTzWQKRtmxaC+=zLTPL-Zkw@mail.gmail.com>
	<CALDaNm2-GJt2HsYTkLqQ=ecm=R-vOBw1=aM_d2EiYbz39x_cTQ@mail.gmail.com>
	<TYCPR01MB8373C3120C2B3112001ED6F1EDCF9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
	<CALDaNm0iZZDB300Dez_97S8G6_RW5QpQ8ef6X3wq8tyK-8wnXQ@mail.gmail.com>
	<CAHut+PtiomM+iyAZHvb2dzfsPvRru266KuBe49hKy2n2h+m_zA@mail.gmail.com>
	<CALDaNm30KDnwX4Czi29fqLb8JBkuwqjbpj9ixwNXXox574NZqQ@mail.gmail.com>
	<CALDaNm1PfKRJsEzbKpyt=v4p3bw+_SzE+LFPsMhR5X+qs+0pPw@mail.gmail.com>
	<TYCPR01MB83730A2F1D6A5303E9C1416AEDD99@TYCPR01MB8373.jpnprd01.prod.outlook.com>
	<CALDaNm0sAU4s1KTLOEWv=rYo5dQK6uFTJn_0FKj3XG1Nv4D-qw@mail.gmail.com>
	<CALDaNm3CLRa95tpas6tEj8x58MUNDShxBNoYS+P8Uq5cryoAOw@mail.gmail.com>
	<CALDaNm0EKC3o=v+F7GneGibuCULGKkBWXmNaVB4GR9HoqD066A@mail.gmail.com>
	<CALDaNm1Z1Rmqj9s6P9ZzmrVA9F_vZ_DwwhYAJmsjqmY6dS3-hA@mail.gmail.com>
	<CAB8KJ=jJGuW=ozKmXZzKDUHZ_-J2ZYGOtJo=i2cnNbSu6=KuYg@mail.gmail.com>
	<CALDaNm1mbFP8fxHU_H1Ex4cT2Aq3n8FE79tq0TO5ThvFnDUYMA@mail.gmail.com>
	<CAB8KJ=jq4RwTs8K7pokmXQwQppP2ChVJLMSAdXaxAX+c1r+mdg@mail.gmail.com>
	<CALDaNm1mJvLni8GODebKBmyegXuZ18bLoG-Pz6H1MCX=vphCYA@mail.gmail.com>
	<CALDaNm3dWZCYDih55qTNAYsjCvYXMFv=46UsDWmfCnXMt3kPCg@mail.gmail.com>
	<CALDaNm1AQZYgT0tALRrkvpP1Q+8+e7vkGCUjQ-jim1C0q3e=zA@mail.gmail.com>
	<CAA4eK1KRdAPC=5=7tQ1GW0cRwD=zaDMi+T4u_k4GxPhPY6e8BQ@mail.gmail.com>
	<OS3PR01MB5718C8BE84B862E7E0CEC29B94BD2@OS3PR01MB5718.jpnprd01.prod.outlook.com>
	<CAA4eK1KYQz7cf46_D=6VkZ4J6Y8vJ88MMi=6zm2TJXDP+V1mLg@mail.gmail.com>
	<CANhcyEXZq4mP5dNgg7u=sMPwvxA4_ZN9U92uZEuzs=0xTu+8Yg@mail.gmail.com>
	<CANhcyEXspT3v5-Tdop9uqQV2HWBvZoN5P0BxXQ6Md6Mr7GXK9A@mail.gmail.com>
	<CAHut+PuiaLOCkiAx9nPnjk6wTbPFvnm9T5svTuKbgwJwTdea8w@mail.gmail.com>
	<CANhcyEV_MePxgftHY65et1WdOAk70M0C7PZ1STPUO8PXHVB1YA@mail.gmail.com>
	<CAHut+Ps0hSNqrjv_jT1AuXxO-CrZue3ixE0jKsxVhtArMrkujQ@mail.gmail.com>
	<CANhcyEXX3viVpYcGHD_fzhf_f6CDQWr2+VBywrJf5zm_XiB4tg@mail.gmail.com>
	<CAHut+PsXP_61ZXuVOx5u9FZGK3oH4taaA59oOzgqyygZx8ezWw@mail.gmail.com>
	<CANhcyEU+aPu6iAH2cTA0cDtn3pd6c_njBONCt3FubYZoEEnm8Q@mail.gmail.com>
	<CAHut+Pv2P6dJ7hZj_fmzN+=xzjvpOpgkAJvDZg3TD2xpvmY1NQ@mail.gmail.com>
	<CANhcyEW2LK4diNeCG862DE40yQoV3VAgf59kXUq2TuR8fnw5vQ@mail.gmail.com>
	<CAHut+PuSHScrODVGCM7P53Mv1HE2N6ThzkH4+gQ1eFXVeD-OCA@mail.gmail.com>
	<CANhcyEUtYV-9ujtxLasnxN_peT+3LuZjcRx1xUECh1CCmANB8w@mail.gmail.com>
	<CAHut+PuviFA6C7qps=+kDYfe3P99as8NCjbR=SYxoi0o96ipoA@mail.gmail.com>
	<CANhcyEXkeg3sjkS3DS9yU1ckz4ozUBNZ+RmrWaRNSSVCR8RquA@mail.gmail.com>
	<CAHut+PsHavMy_KJ0MwR9J6q0BTTty54TxS-KBZc7X6tb4u7rfA@mail.gmail.com>
	<CANhcyEU=k4+0BqOu25N76g738XKUwfLGGdf8e+ssGiRKHC4RwQ@mail.gmail.com>
	<CAHut+Ptdi8txJbMc+5Sp2uB4QLGOp-rZdkBbXyhAkxim0iAhBQ@mail.gmail.com>
	<CANhcyEW+uJB_bvQLEaZCgoRTc1=i+QnrPPHxZ2=0SBSCyj9pkg@mail.gmail.com>
	<CAHut+Ptq0-tMYUOvG3yR34AvuEzR9vUH=muqV_=uEO3zCuA6rA@mail.gmail.com>
	<CANhcyEUEMWSkTfGc7Q3B+UiOzSiOmOGLgK-+C5DXwtCGOnDBhg@mail.gmail.com>
	<CAHut+PuAgSDr3kbSxPMYEyCeGiJ5hgaT1JUvuiYPRT=Q+--O-Q@mail.gmail.com>
	<CANhcyEU_uuiKMRrd_E_DeYsyCvwY_u995E-Do3d66J7tQnzdzw@mail.gmail.com>
	<CANhcyEWK9sj9UY4uaV36Q8qxUv=8Joch0o98RCN5U3xLjUxAag@mail.gmail.com>
	<CALDaNm32XQDR4qsOhPQeophVbZ8r+ShJSSssoVfdPcwG6joPHQ@mail.gmail.com>
	<CANhcyEVt2CBnG7MOktaPPV4rYapHR-VHe5=qoziTZh1L9SVc6w@mail.gmail.com>
	<CAHut+PtGu2j72yV_as_TVKfWr3ctd18svReGEx3xa=q5BtKyKA@mail.gmail.com>
	<CANhcyEUh9ki36VTXyYf8UqUrfLX9ZhfP_f2LjpvvycgqWLQqqQ@mail.gmail.com>
	<CAHut+Pv96u0VciuSx2B99jDHEvn7svVJynCmw-qYb=z4Kc2knA@mail.gmail.com>
	<CANhcyEWGiWwGt1-v6d9bCAae9Np7cNPt+iRV9PXBZ0z=75XEVw@mail.gmail.com>
	<CANhcyEWAQHtUfgNPA2m-+okEh7pXaK5irBm+yzyNVJXL2LUTXw@mail.gmail.com>
	<CAHut+PuzcKXheZwgNvDJkwK5txd1kzNRxCmcJcbr=_9mGHjKtA@mail.gmail.com>
	<CANhcyEXCKPCAdoqBLAhxt64Nwf+7T52dd8daE3qvhBNTvro13Q@mail.gmail.com>
	<CAHut+PsqV+dkPyYZ2BxWKBq7+TkrjCwBHBAQ3bHOSEChovD+ZA@mail.gmail.com>

On Fri, 21 Nov 2025 at 12:26, Peter Smith <[email protected]> wrote:
>
> Hi Shlok.
>
> Here are some review comments for your patch v28-0003 (EXCEPT TABLE ...).
>
> The review of this patch is a WIP. In this post I only looked at the test code.
>
> ======
> .../t/037_rep_changes_except_table.pl
>
> 1.
> +
> +# Copyright (c) 2021-2025, PostgreSQL Global Development Group
> +
> +# Logical replication tests for except table publications
>
> Use uppercase: /except table/EXCEPT TABLE/
>
> ~~~
>
> 2.
> There are lots of test cases dedicated to partiion-table testing. I
> felt a bigger comment separating these major groups might be helpful.
>
> Something like:
>
> -- ============================================
> -- EXCEPT TABLE test cases for normal tables
> -- ============================================
>
> and
>
> -- ============================================
> -- EXCEPT TABLE test cases for partition tables
> -- ============================================
>
> ~~~
>
> 3.
> +# Initialize publisher node
> ...
> +# Create subscriber node
>
> Those 2 comments should be almost alike -- e.g. both should say
> "Initialize" or both should say "Create".
>
> ~~~
>
> 4.
> +# Test replication with publications created using FOR ALL TABLES EXCEPT TABLE
> +# clause.
> +# Create schemas and tables on publisher
> +$node_publisher->safe_psql(
> + 'postgres', qq(
> + CREATE SCHEMA sch1;
> + CREATE TABLE sch1.tab1 AS SELECT generate_series(1,10) AS a;
> + CREATE TABLE public.tab1(a int);
> +));
> +
>
> That first sentence ("Test replication with ...") is not needed here.
> The is just repeating the purpose of the entire file, so that comment
> can replace the one at the top of this file.
>
> ~~~
>
> 5.
> +# Insert some data and verify that inserted data is not replicated
>
> Be explicit that we are referring to the excluded table.
>
> SUGGESTION (e.g.)
> Verify that data inserted to the excluded table is not replcated.
>
> ~~~
>
> 6.
> +# Alter publication to exclude data changes in public.tab1 and verify that
> +# subscriber does not get the changed data for this table.
> +$node_publisher->safe_psql(
> + 'postgres', qq(
> + ALTER PUBLICATION tap_pub_schema RESET;
> + ALTER PUBLICATION tap_pub_schema ADD ALL TABLES EXCEPT TABLE
> (sch1.tab1, public.tab1);
> + INSERT INTO public.tab1 VALUES(generate_series(1,10));
> +));
> +$node_publisher->wait_for_catchup('tap_sub_schema');
> +
>
> It is not strictly needed for these tests, but do you think it makes
> more sense to also do an ALTER SUBSCRIPTION ... REFRESH PUBLICATION;
> whenever you change the publications?
>
> ~~~
>
> 7.
> +# cleanup
> +$node_publisher->safe_psql('postgres', "DROP PUBLICATION tap_pub_schema");
> +$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_schema");
> +
> +
>
> double-blank lines.
>
> ~~~
>
> 8.
> I think it would be more helpful if the partition table test cases say
> (in their comments) a lot more about the steps they are doing, and
> what they expect the result to be. Sure, I can read all the code to
> figure it out for each case, but it is better to know the test
> intentions/expectations then verify they are doing the right thing.
>
> ~~~
>
> 9.
> + CREATE TABLE sch1.t1(a int) PARTITION BY RANGE(a);
> + CREATE TABLE sch1.part1 PARTITION OF sch1.t1 FOR VALUES FROM (0) TO (5);
>
> Maybe create this table to have *multiple* partitions. It might be
> interesting later to see what happens when you try to EXCEPT only one
> of the partitions.
>
I have addressed all the comments
Please find the updated patch in [1].

[1]: https://www.postgresql.org/message-id/CANhcyEXwLrQsec6g%2B1dqWTKyJQMQMh%3Dgetj28C%2BzLL14BjuumA%40ma...

Thanks,
Shlok Kyal





view thread (377+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Skipping schema changes in publication
  In-Reply-To: <CANhcyEXT=DPeULFL2udhcyO1340Y1fGqgpmUqpiCh++JnC8caw@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox