Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1vML42-0076UD-1S for pgsql-hackers@arkaria.postgresql.org; Fri, 21 Nov 2025 06:56:14 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1vML40-0061vP-2q for pgsql-hackers@arkaria.postgresql.org; Fri, 21 Nov 2025 06:56:13 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1vML40-0061vG-1o for pgsql-hackers@lists.postgresql.org; Fri, 21 Nov 2025 06:56:12 +0000 Received: from mail-qk1-x736.google.com ([2607:f8b0:4864:20::736]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.96) (envelope-from ) id 1vML3v-000gJf-2U for pgsql-hackers@lists.postgresql.org; Fri, 21 Nov 2025 06:56:12 +0000 Received: by mail-qk1-x736.google.com with SMTP id af79cd13be357-8b23b6d9f11so161860585a.3 for ; Thu, 20 Nov 2025 22:56:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1763708165; x=1764312965; darn=lists.postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=+s0gRaMPQPxSml2WkxF8nVljKbdLMZW+gSOxZirJEL4=; b=LgcoCl4EG50BEMVcQuNwyN1k3ZoZbuEH4DmuPc0v8I/gDEmOCKpPmHnyEWjNxYVRwy vBeU9vF/fqqxxOdH8KoklzTYlcSTMh0bwCC1YG9aXMyZJ4KWUttOkLYmr7RA9o/0/vJW NrELWmZbh6F8UJiAOcC+6wCfGla5TD69LsvLykHEUkDfinoESLpKG6WviNuYrmwK7tK3 JHm3M10eNbHdj0Ztsy216BNw03SeFTccSYKyqRKiXPjBtz43ngWpE9/NxkYwfWAu3qWg zgQR/YFqsyTV/HTmLWYg/BW05h96W2uPIRvPygllYuEKq3T+FhnMhkXqaFgQknp+K4IV DMFw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763708165; x=1764312965; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=+s0gRaMPQPxSml2WkxF8nVljKbdLMZW+gSOxZirJEL4=; b=J6mbQnT/Le5RM/l+NEHSeAAd8j15nY6RqR4+SJr9bKnakHHuoz1AXIuvL3wYEZ5nle GCC3BUMe3Z6picXCSKotEPHM8K8GND7xztciM6zmxsa2IdODajqa+S7Ro4FgIRUumTYx 26qe1uupC4Zu+AkHjp3ASvvGGhTNC/zlcsmZ7/dsBiVkIm/FPLwwnLeUMdaci3AEl7mW BE7buqLgSuMLcus5bGqmhxCU7nX0Mua+I126ZpuHQhFOT1MBkZfuKow95UaGaj1Ri8xI 2i1nOkWGiX06QoM5ckiP7PegtwasMbNp4ag8fIcvUFyb1CUX2DVRFBKxywkjmBqNS8nS 1s8A== X-Forwarded-Encrypted: i=1; AJvYcCWzmwACVvqtp1W3tQrAp+611vayrcDiR7ZdeQR4f3jiwfV/gKPvdQyNLOI8stBpBdjjU15LnTKQbTYNeSze@lists.postgresql.org X-Gm-Message-State: AOJu0Yzo7yfpENNK9RuWK0im/tURS/hyFV6o/Mocy+7d0wYJ/kklqEdh 6ywbpxN1KZSc8uPDTxKgBI3SaEdJz7Qk/x8ChDJoWhNCa9z/+XkKptZ9saj2YDi4xNsDoyr3989 d1DfveWlw3ebz+irKsMvF/45KbzKYibw= X-Gm-Gg: ASbGncvKY0iO091gx3GaIJBxxJpapM3HfuKc6OZEQ+cvfsdCikYJzZ6U+Lm2Mtugs4G IUuuNrloA5s93tenfwyC25rq/JfmzH2l9KtWA41LSIJT+juUqf4Huc8O5LkA1uPTM1Rw2s8LrD/ axJ01nbc4qEXPS+b/Xwui5a2PT7D61Cw9JwVhIQD8chb8w52xby2p05lnVo0runy6AauHVe2gMS Ze1z/LSfcahORw+gavClnaOzafoHoeFGd45NxMekga0tb9D64gxhqTf8pCNgnwgI68r6OwMsj/f qEws+MSlR1TncxdWGs+zk54LzFF2mA== X-Google-Smtp-Source: AGHT+IG2YdDOjth9CFCnqetJBhXMH5jU7SdQ3KbswAeb85olds2GZdlnzxarCnPmA/6loYk3jdK+gv/Ts4NFkJN1Jv4= X-Received: by 2002:a05:620a:45a7:b0:8a3:cd9e:e40d with SMTP id af79cd13be357-8b33d483825mr105956485a.88.1763708165336; Thu, 20 Nov 2025 22:56:05 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Peter Smith Date: Fri, 21 Nov 2025 17:55:37 +1100 X-Gm-Features: AWmQ_bnKscKaCUsyXYVsd6IiRaT5XKAXJJqpg8RHOeyuHpd-ITwEqsvJv_-196Y Message-ID: Subject: Re: Skipping schema changes in publication To: Shlok Kyal Cc: vignesh C , Amit Kapila , "Zhijie Hou (Fujitsu)" , YeXiu <1518981153@qq.com>, Ian Lawrence Barwick , Bharath Rupireddy , PostgreSQL Hackers Content-Type: text/plain; charset="UTF-8" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk 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. ====== Kind Regards, Peter Smith. Fujitsu Australia