public inbox for [email protected]  
help / color / mirror / Atom feed
From: Peter Eisentraut <[email protected]>
To: Tomas Vondra <[email protected]>
To: Amit Kapila <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Justin Pryzby <[email protected]>
Cc: Rahila Syed <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: Column Filtering in Logical Replication
Date: Thu, 24 Mar 2022 17:33:56 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CAA4eK1KONbrck=hQTFSFgwio5rqu-82KZba2HLE_AcVa_RWYcQ@mail.gmail.com>
	<[email protected]>
	<CAA4eK1JyoVty_BZOMJdx7VMkBTadvv8oOYrh9xTQ5BY+k-3+OA@mail.gmail.com>
	<[email protected]>
	<CAA4eK1JJ5qgqbjfYQU9F4R1+OABxec-DQechQXus0fanDjaAVg@mail.gmail.com>
	<OS0PR01MB571614E4A39DD33669D5AFD6940A9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAA4eK1J9UDnM-xN7viu4eO_DDL_PqMG9F6ZsskqEU7snujXi+A@mail.gmail.com>
	<[email protected]>
	<CAA4eK1KUpOnFam+2bBuGGX-_pZbmO_ShJV6G=GT-RyM7gQSvWg@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>


On 17.03.22 20:11, Tomas Vondra wrote:
> But the comment describes the error for the whole block, which looks
> like this:
> 
> -- error: replica identity "a" not included in the column list
> ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (b, c);
> UPDATE testpub_tbl5 SET a = 1;
> ERROR:  cannot update table "testpub_tbl5"
> DETAIL:  Column list used by the publication does not cover the replica
> identity.
> 
> So IMHO the comment is correct.

Ok, that makes sense.  I read all the comments in the test file again. 
There were a couple that I think could use tweaking; see attached file. 
The ones with "???" didn't make sense to me:  The first one is before a 
command that doesn't seem to change anything, the second one I didn't 
understand the meaning.  Please take a look.

(The patch is actually based on your 20220318c patch, but I'm adding it 
here since we have the discussion here.)
From 2e6352791e5418bb0726a051660d44311046fc28 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Thu, 24 Mar 2022 17:30:32 +0100
Subject: [PATCH] fixup! Allow specifying column lists for logical replication

---
 src/test/regress/sql/publication.sql | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index aeb1b572af..d50052ef9d 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -399,14 +399,14 @@ CREATE TABLE testpub_tbl5 (a int PRIMARY KEY, b text, c text,
 -- ok
 ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, c);
 ALTER TABLE testpub_tbl5 DROP COLUMN c;		-- no dice
--- ok: for insert-only publication, the column list is arbitrary
+-- ok: for insert-only publication, any column list is acceptable
 ALTER PUBLICATION testpub_fortable_insert ADD TABLE testpub_tbl5 (b, c);
 
 /* not all replica identities are good enough */
 CREATE UNIQUE INDEX testpub_tbl5_b_key ON testpub_tbl5 (b, c);
 ALTER TABLE testpub_tbl5 ALTER b SET NOT NULL, ALTER c SET NOT NULL;
 ALTER TABLE testpub_tbl5 REPLICA IDENTITY USING INDEX testpub_tbl5_b_key;
--- error: replica identity (b,c) is covered by column list (a, c)
+-- error: replica identity (b,c) is not covered by column list (a, c)
 UPDATE testpub_tbl5 SET a = 1;
 ALTER PUBLICATION testpub_fortable DROP TABLE testpub_tbl5;
 
@@ -423,7 +423,7 @@ CREATE PUBLICATION testpub_table_ins WITH (publish = 'insert, truncate');
 ALTER PUBLICATION testpub_table_ins ADD TABLE testpub_tbl5 (a);		-- ok
 \dRp+ testpub_table_ins
 
--- with REPLICA IDENTITY FULL, column lists are not allowed
+-- tests with REPLICA IDENTITY FULL
 CREATE TABLE testpub_tbl6 (a int, b text, c text);
 ALTER TABLE testpub_tbl6 REPLICA IDENTITY FULL;
 
@@ -434,11 +434,11 @@ CREATE TABLE testpub_tbl6 (a int, b text, c text);
 ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl6; -- ok
 UPDATE testpub_tbl6 SET a = 1;
 
--- make sure changing the column list is updated in SET TABLE
+-- make sure changing the column list is updated in SET TABLE ???
 CREATE TABLE testpub_tbl7 (a int primary key, b text, c text);
 ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl7 (a, b);
 \d+ testpub_tbl7
--- ok: we'll skip this table
+-- ok: we'll skip this table ???
 ALTER PUBLICATION testpub_fortable SET TABLE testpub_tbl7 (a, b);
 \d+ testpub_tbl7
 -- ok: update the column list
-- 
2.35.1



Attachments:

  [text/plain] 0001-fixup-Allow-specifying-column-lists-for-logical-repl.patch (2.4K, ../[email protected]/2-0001-fixup-Allow-specifying-column-lists-for-logical-repl.patch)
  download | inline diff:
From 2e6352791e5418bb0726a051660d44311046fc28 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Thu, 24 Mar 2022 17:30:32 +0100
Subject: [PATCH] fixup! Allow specifying column lists for logical replication

---
 src/test/regress/sql/publication.sql | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index aeb1b572af..d50052ef9d 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -399,14 +399,14 @@ CREATE TABLE testpub_tbl5 (a int PRIMARY KEY, b text, c text,
 -- ok
 ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, c);
 ALTER TABLE testpub_tbl5 DROP COLUMN c;		-- no dice
--- ok: for insert-only publication, the column list is arbitrary
+-- ok: for insert-only publication, any column list is acceptable
 ALTER PUBLICATION testpub_fortable_insert ADD TABLE testpub_tbl5 (b, c);
 
 /* not all replica identities are good enough */
 CREATE UNIQUE INDEX testpub_tbl5_b_key ON testpub_tbl5 (b, c);
 ALTER TABLE testpub_tbl5 ALTER b SET NOT NULL, ALTER c SET NOT NULL;
 ALTER TABLE testpub_tbl5 REPLICA IDENTITY USING INDEX testpub_tbl5_b_key;
--- error: replica identity (b,c) is covered by column list (a, c)
+-- error: replica identity (b,c) is not covered by column list (a, c)
 UPDATE testpub_tbl5 SET a = 1;
 ALTER PUBLICATION testpub_fortable DROP TABLE testpub_tbl5;
 
@@ -423,7 +423,7 @@ CREATE PUBLICATION testpub_table_ins WITH (publish = 'insert, truncate');
 ALTER PUBLICATION testpub_table_ins ADD TABLE testpub_tbl5 (a);		-- ok
 \dRp+ testpub_table_ins
 
--- with REPLICA IDENTITY FULL, column lists are not allowed
+-- tests with REPLICA IDENTITY FULL
 CREATE TABLE testpub_tbl6 (a int, b text, c text);
 ALTER TABLE testpub_tbl6 REPLICA IDENTITY FULL;
 
@@ -434,11 +434,11 @@ CREATE TABLE testpub_tbl6 (a int, b text, c text);
 ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl6; -- ok
 UPDATE testpub_tbl6 SET a = 1;
 
--- make sure changing the column list is updated in SET TABLE
+-- make sure changing the column list is updated in SET TABLE ???
 CREATE TABLE testpub_tbl7 (a int primary key, b text, c text);
 ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl7 (a, b);
 \d+ testpub_tbl7
--- ok: we'll skip this table
+-- ok: we'll skip this table ???
 ALTER PUBLICATION testpub_fortable SET TABLE testpub_tbl7 (a, b);
 \d+ testpub_tbl7
 -- ok: update the column list
-- 
2.35.1



view thread (185+ 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: Column Filtering in Logical Replication
  In-Reply-To: <[email protected]>

* 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