On Sat, Jan 16, 2021 at 12:21 PM Bharath Rupireddy
<bharath.rupireddyforpostg...@gmail.com> wrote:
> > In the test, can we have multiple publications for the subscription
> > because that is how we discovered that the originally proposed patch
> > was not correct? Also, is it possible to extend some of the existing
> > tests in 001_rep_changes.pl or anywhere so that we can avoid some cost
> > of the replication setup.
>
> Sure, I will add the multiple publications use case provided by japin
> and also see if I can move the tests from 100_bugs.pl to
> 0001_rep_changes.pl.
Attaching v5 patch set. 0001 - No change. 0002 - I moved the tests to
001_rep_changes.pl so that we could avoid creation of subscriber,
publisher nodes and other set up. I also added the multiple
publication for the subscription test case which was failing with
v2-0001 patch. Note that I had to create subscriber, publications for
this test case, because I couldn't find the regression (on v2-001
patch) with any of the existing test cases and also I'm dropping them
after the test so that it will not harm any existing following tests.
Hope that's okay. With v5-0002 and v2-0001, the multiple publication
for the subscription test case fails.
Please consider the v5 patch set for further review.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
From 7dc6f91ca7225119b4fe6b3f0869385519721d2c Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Date: Sat, 16 Jan 2021 11:30:09 +0530
Subject: [PATCH v5 1/1] Fix ALTER PUBLICATION...DROP TABLE behaviour
Currently, in logical replication, publisher/walsender publishes
the tables even though they aren't part of the publication i.e
they are dropped from the publication. Because of this ALTER
PUBLICATION...DROP TABLE doesn't work as expected.
To fix above issue, when the entry got invalidated in
rel_sync_cache_publication_cb(), mark the pubactions to false
and let get_rel_sync_entry() recalculate the pubactions.
---
src/backend/replication/pgoutput/pgoutput.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index 2f01137b42..478cd1f9f5 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -1179,5 +1179,18 @@ rel_sync_cache_publication_cb(Datum arg, int cacheid, uint32 hashvalue)
*/
hash_seq_init(&status, RelationSyncCache);
while ((entry = (RelationSyncEntry *) hash_seq_search(&status)) != NULL)
+ {
entry->replicate_valid = false;
+
+ /*
+ * There might be some relations dropped from the publication, we do
+ * not need to publish the changes for them. Since we cannot get the
+ * the dropped relations (see above), we reset pubactions for all
+ * entries.
+ */
+ entry->pubactions.pubinsert = false;
+ entry->pubactions.pubupdate = false;
+ entry->pubactions.pubdelete = false;
+ entry->pubactions.pubtruncate = false;
+ }
}
--
2.25.1
From 95f4855a1f7ecaa8e2897a154e9347945770e76c Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Date: Sat, 16 Jan 2021 17:13:06 +0530
Subject: [PATCH v5 2/2] Test behaviour of ALTER PUBLICATION ... DROP TABLE
---
src/test/subscription/t/001_rep_changes.pl | 95 +++++++++++++++++++++-
1 file changed, 94 insertions(+), 1 deletion(-)
diff --git a/src/test/subscription/t/001_rep_changes.pl b/src/test/subscription/t/001_rep_changes.pl
index 0680f44a1a..215ccf40c9 100644
--- a/src/test/subscription/t/001_rep_changes.pl
+++ b/src/test/subscription/t/001_rep_changes.pl
@@ -3,7 +3,7 @@ use strict;
use warnings;
use PostgresNode;
use TestLib;
-use Test::More tests => 23;
+use Test::More tests => 27;
# Initialize publisher node
my $node_publisher = get_new_node('publisher');
@@ -153,6 +153,99 @@ is($result, qq(20|-20|-1),
$node_publisher->safe_psql('postgres',
"INSERT INTO tab_full SELECT generate_series(1,10)");
+# Test behaviour of ALTER PUBLICATION ... DROP TABLE
+
+# When publisher drops a table from publication, it should also stop sending
+# its changes to subscriber. We look at the subscriber whether it receives
+# the row that is inserted to the table in the publisher after it is dropped
+# from the publication.
+
+$result = $node_subscriber->safe_psql('postgres',
+ "SELECT count(*), min(a), max(a) FROM tab_ins");
+is($result, qq(1052|1|1002), 'check rows on subscriber before table drop from publication');
+
+# Drop the table from publicaiton
+$node_publisher->safe_psql('postgres',
+ "ALTER PUBLICATION tap_pub_ins_only DROP TABLE tab_ins");
+
+# Insert a row in publisher, but publisher will not send this row to subscriber
+$node_publisher->safe_psql('postgres', "INSERT INTO tab_ins VALUES(8888)");
+
+$node_publisher->wait_for_catchup('tap_sub');
+
+# Subscriber will not receive the inserted row, after table is dropped from
+# publication, so row count should remain the same.
+$result = $node_subscriber->safe_psql('postgres',
+ "SELECT count(*), min(a), max(a) FROM tab_ins");
+is($result, qq(1052|1|1002), 'check rows on subscriber after table drop from publication');
+
+# Delete the inserted row in publisher
+$node_publisher->safe_psql('postgres', "DELETE FROM tab_ins WHERE a = 8888");
+
+# Add the table to publicaiton again
+$node_publisher->safe_psql('postgres',
+ "ALTER PUBLICATION tap_pub_ins_only ADD TABLE tab_ins");
+
+# Rerfresh publication after table is added to publication
+$node_subscriber->safe_psql('postgres',
+ "ALTER SUBSCRIPTION tap_sub REFRESH PUBLICATION");
+
+# Test replication with multiple publications for subscription
+
+# Create tables on publisher
+$node_publisher->safe_psql('postgres', "CREATE TABLE temp1 (a int)");
+$node_publisher->safe_psql('postgres', "CREATE TABLE temp2 (a int)");
+
+# Create tables on subscriber
+$node_subscriber->safe_psql('postgres', "CREATE TABLE temp1 (a int)");
+$node_subscriber->safe_psql('postgres', "CREATE TABLE temp2 (a int)");
+
+# Setup logical replication that will only be used for this test
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION tap_pub_temp1 FOR TABLE temp1 WITH (publish = insert)");
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION tap_pub_temp2 FOR TABLE temp2");
+$node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION tap_sub_temp1 CONNECTION '$publisher_connstr' PUBLICATION tap_pub_temp1, tap_pub_temp2"
+);
+
+$node_publisher->wait_for_catchup('tap_sub_temp1');
+
+# Also wait for initial table sync to finish
+$synced_query =
+ "SELECT count(1) = 0 FROM pg_subscription_rel WHERE srsubstate NOT IN ('r', 's');";
+$node_subscriber->poll_query_until('postgres', $synced_query)
+ or die "Timed out while waiting for subscriber to synchronize data";
+
+# Subscriber table will have no rows initially
+$result = $node_subscriber->safe_psql('postgres',
+ "SELECT count(*) FROM temp1");
+is($result, qq(0), 'check initial rows on subscriber with multiple publications');
+
+# Insert a row into the table that's part of first publication in subscriber
+# list of publications.
+$node_publisher->safe_psql('postgres', "INSERT INTO temp1 VALUES (1)");
+
+$node_publisher->wait_for_catchup('tap_sub_temp1');
+
+# Subscriber should receive the inserted row
+$result = $node_subscriber->safe_psql('postgres',
+ "SELECT count(*) FROM temp1");
+is($result, qq(1), 'check rows on subscriber with multiple publications');
+
+# Drop subscription as we don't need it anymore
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_temp1");
+
+# Drop publications as we don't need them anymore
+$node_publisher->safe_psql('postgres', "DROP PUBLICATION tap_pub_temp1");
+$node_publisher->safe_psql('postgres', "DROP PUBLICATION tap_pub_temp2");
+
+# Clean up the tables on both publisher and subscriber as we don't need them
+$node_publisher->safe_psql('postgres', "DROP TABLE temp1");
+$node_publisher->safe_psql('postgres', "DROP TABLE temp2");
+$node_subscriber->safe_psql('postgres', "DROP TABLE temp1");
+$node_subscriber->safe_psql('postgres', "DROP TABLE temp2");
+
# add REPLICA IDENTITY FULL so we can update
$node_publisher->safe_psql('postgres',
"ALTER TABLE tab_full REPLICA IDENTITY FULL");
--
2.25.1
