On 02.12.21 15:23, Alvaro Herrera wrote:
I took the latest posted patch, rebased on current sources, fixed the
conflicts, and pgindented. No further changes. Here's the result. All
tests are passing for me. Some review comments that were posted have
not been addressed yet; I'll look into that soon.
In v7 I have reinstated the test file and fixed the silly problem that
caused it to fail (probably a mistake of mine while rebasing).
I looked through this a bit. You had said that you are still going to
integrate past review comments, so I didn't look to deeply before you
get to that.
Attached are a few fixup patches that you could integrate.
There was no documentation, so I wrote a bit (patch 0001). It only
touches the CREATE PUBLICATION and ALTER PUBLICATION pages at the
moment. There was no mention in the Logical Replication chapter that
warranted updating. Perhaps we should revisit that chapter at the end
of the release cycle.
DDL tests should be done in src/test/regress/sql/publication.sql rather
than through TAP tests, to keep it simpler. I have added a few that I
came up with (patch 0002). Note the FIXME marker that it does not
recognize if the listed columns don't exist. I removed a now redundant
test from the TAP test file. The other error condition test in the TAP
test file ('publication relation test_part removed') I didn't
understand: test_part was added with columns (a, b), so why would
dropping column b remove the whole entry? Maybe I missed something, or
this could be explained better.
I was curious what happens when you have different publications with
different column lists, so I wrote a test for that (patch 0003). It
turns out it works, so there is nothing to do, but perhaps the test is
useful to keep.
The test file 021_column_filter.pl should be renamed to an unused number
(would be 027 currently). Also, it contains references to "TRUNCATE",
where it was presumably copied from.
On the implementation side, I think the added catalog column
pg_publication_rel.prattrs should be an int2 array, not a text array.
That would also fix the above problem. If you have to look up the
columns at DDL time, then you will notice when they don't exist.
Finally, I suggest not naming this feature "column filter". I think
this name arose because of the analogy with the "row filter" feature
also being developed. But a filter is normally a dynamic data-driven
action, which this is not. Golden Gate calls it in their documentation
"Selecting Columns", or we could just call it "column list".From cc9082fe6f98e5d9d992377761d06b8aadf3b27f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Fri, 10 Dec 2021 13:19:52 +0100
Subject: [PATCH 1/3] Add some documentation
---
doc/src/sgml/ref/alter_publication.sgml | 4 +++-
doc/src/sgml/ref/create_publication.sgml | 11 ++++++++++-
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/ref/alter_publication.sgml
b/doc/src/sgml/ref/alter_publication.sgml
index bb4ef5e5e2..c86055b93c 100644
--- a/doc/src/sgml/ref/alter_publication.sgml
+++ b/doc/src/sgml/ref/alter_publication.sgml
@@ -30,7 +30,7 @@
<phrase>where <replaceable class="parameter">publication_object</replaceable>
is one of:</phrase>
- TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ *
] [, ... ]
+ TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ *
] [ ( <replaceable class="parameter">column_name</replaceable>, [, ... ] ) ]
[, ... ]
ALL TABLES IN SCHEMA { <replaceable
class="parameter">schema_name</replaceable> | CURRENT_SCHEMA } [, ... ]
</synopsis>
</refsynopsisdiv>
@@ -110,6 +110,8 @@ <title>Parameters</title>
specified, the table and all its descendant tables (if any) are
affected. Optionally, <literal>*</literal> can be specified after the
table
name to explicitly indicate that descendant tables are included.
+ Optionally, a column list can be specified. See <xref
+ linkend="sql-createpublication"/> for details.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/create_publication.sgml
b/doc/src/sgml/ref/create_publication.sgml
index d805e8e77a..73a23cbb02 100644
--- a/doc/src/sgml/ref/create_publication.sgml
+++ b/doc/src/sgml/ref/create_publication.sgml
@@ -28,7 +28,7 @@
<phrase>where <replaceable class="parameter">publication_object</replaceable>
is one of:</phrase>
- TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ *
] [, ... ]
+ TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ *
] [ ( <replaceable class="parameter">column_name</replaceable>, [, ... ] ) ] [,
... ]
ALL TABLES IN SCHEMA { <replaceable
class="parameter">schema_name</replaceable> | CURRENT_SCHEMA } [, ... ]
</synopsis>
</refsynopsisdiv>
@@ -78,6 +78,15 @@ <title>Parameters</title>
publication, so they are never explicitly added to the publication.
</para>
+ <para>
+ When a column list is specified, only the listed columns are replicated;
+ any other columns are ignored for the purpose of replication through
+ this publication. If no column list is specified, all columns of the
+ table are replicated through this publication, including any columns
+ added later. If a column list is specified, it must include the replica
+ identity columns.
+ </para>
+
<para>
Only persistent base tables and partitioned tables can be part of a
publication. Temporary tables, unlogged tables, foreign tables,
--
2.34.1
From dadc41b139c352811f5956892c02135905263347 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Fri, 10 Dec 2021 13:40:50 +0100
Subject: [PATCH 2/3] Add regression tests
---
src/test/regress/expected/publication.out | 16 +++++++++++++++-
src/test/regress/sql/publication.sql | 12 +++++++++++-
src/test/subscription/t/021_column_filter.pl | 6 +-----
3 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/src/test/regress/expected/publication.out
b/src/test/regress/expected/publication.out
index 5ac2d666a2..c6017c0514 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -165,7 +165,21 @@ Publications:
regress_publication_user | t | t | t | f | f
| f
(1 row)
-DROP TABLE testpub_tbl2;
+CREATE TABLE testpub_tbl5 (a int PRIMARY KEY, b text, c text);
+ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (x, y, z); -- error
+ERROR: cannot add relation "testpub_tbl5" to publication
+DETAIL: Column filter must include REPLICA IDENTITY columns
+ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, x); -- error
FIXME
+ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (b, c); -- error
+ERROR: relation "testpub_tbl5" is already member of publication
"testpub_fortable"
+ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, c); -- ok
+ERROR: relation "testpub_tbl5" is already member of publication
"testpub_fortable"
+CREATE TABLE testpub_tbl6 (a int, b text, c text);
+ALTER TABLE testpub_tbl6 REPLICA IDENTITY FULL;
+ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl6 (a, b, c); -- error
+ERROR: cannot add relation "testpub_tbl6" to publication
+DETAIL: Cannot have column filter with REPLICA IDENTITY FULL
+DROP TABLE testpub_tbl2, testpub_tbl5, testpub_tbl6;
DROP PUBLICATION testpub_foralltables, testpub_fortable, testpub_forschema;
CREATE TABLE testpub_tbl3 (a int);
CREATE TABLE testpub_tbl3a (b text) INHERITS (testpub_tbl3);
diff --git a/src/test/regress/sql/publication.sql
b/src/test/regress/sql/publication.sql
index 56dd358554..b2fd793c61 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -89,7 +89,17 @@ CREATE PUBLICATION testpub_for_tbl_schema FOR ALL TABLES IN
SCHEMA pub_test, TAB
\d+ testpub_tbl2
\dRp+ testpub_foralltables
-DROP TABLE testpub_tbl2;
+CREATE TABLE testpub_tbl5 (a int PRIMARY KEY, b text, c text);
+ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (x, y, z); -- error
+ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, x); -- error
FIXME
+ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (b, c); -- error
+ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, c); -- ok
+
+CREATE TABLE testpub_tbl6 (a int, b text, c text);
+ALTER TABLE testpub_tbl6 REPLICA IDENTITY FULL;
+ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl6 (a, b, c); -- error
+
+DROP TABLE testpub_tbl2, testpub_tbl5, testpub_tbl6;
DROP PUBLICATION testpub_foralltables, testpub_fortable, testpub_forschema;
CREATE TABLE testpub_tbl3 (a int);
diff --git a/src/test/subscription/t/021_column_filter.pl
b/src/test/subscription/t/021_column_filter.pl
index 7b52d4593d..02e05420f1 100644
--- a/src/test/subscription/t/021_column_filter.pl
+++ b/src/test/subscription/t/021_column_filter.pl
@@ -5,7 +5,7 @@
use warnings;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 9;
+use Test::More tests => 8;
# setup
@@ -130,10 +130,6 @@
is($result, qq(1|abc), 'update on column c is not replicated');
#Test error conditions
-my ($psql_rc, $psql_out, $psql_err) = $node_publisher->psql('postgres',
- "CREATE PUBLICATION pub2 FOR TABLE test_part(b)");
-like($psql_err, qr/Column filter must include REPLICA IDENTITY columns/,
'Error when column filter does not contain REPLICA IDENTITY');
-
$node_publisher->safe_psql('postgres',
"ALTER TABLE test_part DROP COLUMN b");
$result = $node_publisher->safe_psql('postgres',
--
2.34.1
From 8f8307000fcd047f3a11ddc4366e6fd386908296 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Fri, 10 Dec 2021 13:51:38 +0100
Subject: [PATCH 3/3] Test case for overlapping column lists
---
src/test/subscription/t/021_column_filter.pl | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/src/test/subscription/t/021_column_filter.pl
b/src/test/subscription/t/021_column_filter.pl
index 02e05420f1..3814ad2367 100644
--- a/src/test/subscription/t/021_column_filter.pl
+++ b/src/test/subscription/t/021_column_filter.pl
@@ -5,7 +5,7 @@
use warnings;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 8;
+use Test::More tests => 9;
# setup
@@ -137,3 +137,18 @@
is($result, qq(tab1|{a,B}
tab2|{a,b}
tab3|{a',c'}), 'publication relation test_part removed');
+
+
+$node_publisher->safe_psql('postgres', "CREATE TABLE tab4 (a int PRIMARY KEY,
b int, c int, d int)");
+$node_subscriber->safe_psql('postgres', "CREATE TABLE tab4 (a int PRIMARY KEY,
b int, d int)");
+$node_publisher->safe_psql('postgres', "CREATE PUBLICATION pub2 FOR TABLE tab4
(a, b)");
+$node_publisher->safe_psql('postgres', "CREATE PUBLICATION pub3 FOR TABLE tab4
(a, d)");
+$node_subscriber->safe_psql('postgres', "CREATE SUBSCRIPTION sub2
CONNECTION '$publisher_connstr' PUBLICATION pub2, pub3");
+$node_publisher->wait_for_catchup('sub2');
+$node_publisher->safe_psql('postgres', "INSERT INTO tab4 VALUES (1, 11, 111,
1111)");
+$node_publisher->safe_psql('postgres', "INSERT INTO tab4 VALUES (2, 22, 222,
2222)");
+$node_publisher->wait_for_catchup('sub2');
+is($node_subscriber->safe_psql('postgres',"SELECT * FROM tab4;"),
+ qq(1|11|1111
+2|22|2222),
+ 'overlapping publications with overlapping column lists');
--
2.34.1
