Ignore heap rewrites for materialized views in logical replication

From fe13dc44f210b5f14b97bf9a29a242c2211ba700 Mon Sep 17 00:00:00 2001
From: Euler Taveira <euler.taveira@enterprisedb.com>
Date: Fri, 27 May 2022 11:35:27 -0300
Subject: [PATCH v2 1/2] Ignore heap rewrites for materialized views in logical
 replication

If you have a publication that specifies FOR ALL TABLES clause, a REFRESH
MATERIALIZED VIEW can break your setup with the following message

ERROR:  logical replication target relation "public.pg_temp_NNN" does not exist

Commit 1a499c2520 introduces a heuristic to prevent that transient heaps (due
to table rewrites) are included for a FOR ALL TABLES publication. However, it
forgot to exclude materialized views (heap rewrites occurs when you execute
REFRESH MATERIALIZED VIEW). Since materialized views are not supported in
logical decoding, it is appropriate to filter them too.

As explained in the commit message, a more robust solution was included in
version 11 (commit 325f2ec555), hence only version 10 is affected.
---
 src/backend/replication/pgoutput/pgoutput.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index f96fde3df0..e03ff4a11e 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -540,7 +540,9 @@ get_rel_sync_entry(PGOutputData *data, Oid relid)
 				if (sscanf(relname, "pg_temp_%u%n", &u, &n) == 1 &&
 					relname[n] == '\0')
 				{
-					if (get_rel_relkind(u) == RELKIND_RELATION)
+					char	relkind = get_rel_relkind(u);
+
+					if (relkind == RELKIND_RELATION || relkind == RELKIND_MATVIEW)
 						break;
 				}
 			}
-- 
2.30.2

From 04b6b6566f4cf4ddc8cf3ddc51420e93c599024d Mon Sep 17 00:00:00 2001
From: Euler Taveira <euler.taveira@enterprisedb.com>
Date: Fri, 27 May 2022 14:43:10 -0300
Subject: [PATCH v2 2/2] test: heap rewrite for materialized view in logical
 replication

---
 src/test/subscription/t/006_rewrite.pl | 35 +++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/src/test/subscription/t/006_rewrite.pl b/src/test/subscription/t/006_rewrite.pl
index 5e3211aefa..27eb6a1cb7 100644
--- a/src/test/subscription/t/006_rewrite.pl
+++ b/src/test/subscription/t/006_rewrite.pl
@@ -3,7 +3,8 @@ use strict;
 use warnings;
 use PostgresNode;
 use TestLib;
-use Test::More tests => 2;
+use Time::HiRes qw(usleep);
+use Test::More tests => 3;
 
 sub wait_for_caught_up
 {
@@ -26,6 +27,13 @@ my $ddl = "CREATE TABLE test1 (a int, b text);";
 $node_publisher->safe_psql('postgres', $ddl);
 $node_subscriber->safe_psql('postgres', $ddl);
 
+$ddl = "CREATE TABLE test2 (a int, b text);";
+$node_publisher->safe_psql('postgres', $ddl);
+$node_subscriber->safe_psql('postgres', $ddl);
+
+$node_publisher->safe_psql('postgres', q{INSERT INTO test2 (a, b) VALUES (10, 'ten'), (20, 'twenty');});
+$node_publisher->safe_psql('postgres', 'CREATE MATERIALIZED VIEW test3 AS SELECT a, b FROM test2;');
+
 my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
 my $appname           = 'encoding_test';
 
@@ -69,5 +77,30 @@ is($node_subscriber->safe_psql('postgres', q{SELECT a, b, c FROM test1}),
 3|three|33),
    'data replicated to subscriber');
 
+$node_publisher->safe_psql('postgres', 'REFRESH MATERIALIZED VIEW test3;');
+
+# an additional row to check if the REFRESH worked
+$node_publisher->safe_psql('postgres', q{INSERT INTO test2 (a, b) VALUES (30, 'thirty');});
+
+my $max_attempts = 10 * $TestLib::timeout_default;
+my $attempts     = 0;
+my $ret          = 0;
+my $errmsg = qr/logical replication target relation.*does not exist/;
+while ($attempts < $max_attempts)
+{
+	# Succeed. Bail out.
+	if ($node_subscriber->safe_psql('postgres', 'SELECT COUNT(1) FROM test2') == 3) {
+		$ret = 1;
+		last;
+	}
+
+	# Failed. Bail out.
+	last if (slurp_file($node_subscriber->logfile) =~ $errmsg);
+
+	usleep(100_000);
+	$attempts++;
+}
+is($ret, 1, 'heap rewrite for materialized view on subscriber');
+
 $node_subscriber->stop;
 $node_publisher->stop;
-- 
2.30.2

From ae56052c01cc04d4a8d197a829ada515a2e9f69a Mon Sep 17 00:00:00 2001
From: Amit Kapila <akapila@postgresql.org>
Date: Tue, 31 May 2022 19:29:27 +0530
Subject: [PATCH v3] Ignore heap rewrites for materialized views in logical
 replication.

If you have a publication that specifies FOR ALL TABLES clause, a REFRESH
MATERIALIZED VIEW can break your setup with the following message

ERROR: logical replication target relation "public.pg_temp_NNN" does not exist

Commit 1a499c2520 introduces a heuristic to skip table writes that look
like they are from a heap rewrite for a FOR ALL TABLES publication.
However, it forgot to exclude materialized views (heap rewrites occur when
you execute REFRESH MATERIALIZED VIEW). Since materialized views are not
supported in logical decoding, it is appropriate to filter them too.

As explained in the commit 1a499c2520, a more robust solution was included
in version 11 (commit 325f2ec555), hence only version 10 is affected.

Reported-by: Euler Taveira
Author: Euler Taveira
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/bc557ebe-92dc-4afa-b6bb-285a9eeaa614@www.fastmail.com
---
 src/backend/replication/pgoutput/pgoutput.c |  4 +++-
 src/test/subscription/t/006_rewrite.pl      | 20 +++++++++++++++++++-
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index f96fde3..e03ff4a 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -540,7 +540,9 @@ get_rel_sync_entry(PGOutputData *data, Oid relid)
 				if (sscanf(relname, "pg_temp_%u%n", &u, &n) == 1 &&
 					relname[n] == '\0')
 				{
-					if (get_rel_relkind(u) == RELKIND_RELATION)
+					char	relkind = get_rel_relkind(u);
+
+					if (relkind == RELKIND_RELATION || relkind == RELKIND_MATVIEW)
 						break;
 				}
 			}
diff --git a/src/test/subscription/t/006_rewrite.pl b/src/test/subscription/t/006_rewrite.pl
index 5e3211a..20151b2 100644
--- a/src/test/subscription/t/006_rewrite.pl
+++ b/src/test/subscription/t/006_rewrite.pl
@@ -3,7 +3,7 @@ use strict;
 use warnings;
 use PostgresNode;
 use TestLib;
-use Test::More tests => 2;
+use Test::More tests => 3;
 
 sub wait_for_caught_up
 {
@@ -26,6 +26,13 @@ my $ddl = "CREATE TABLE test1 (a int, b text);";
 $node_publisher->safe_psql('postgres', $ddl);
 $node_subscriber->safe_psql('postgres', $ddl);
 
+$ddl = "CREATE TABLE test2 (a int, b text);";
+$node_publisher->safe_psql('postgres', $ddl);
+$node_subscriber->safe_psql('postgres', $ddl);
+
+$node_publisher->safe_psql('postgres', q{INSERT INTO test2 (a, b) VALUES (10, 'ten'), (20, 'twenty');});
+$node_publisher->safe_psql('postgres', 'CREATE MATERIALIZED VIEW test3 AS SELECT a, b FROM test2;');
+
 my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
 my $appname           = 'encoding_test';
 
@@ -69,5 +76,16 @@ is($node_subscriber->safe_psql('postgres', q{SELECT a, b, c FROM test1}),
 3|three|33),
    'data replicated to subscriber');
 
+# Another DDL that causes a heap rewrite
+$node_publisher->safe_psql('postgres', 'REFRESH MATERIALIZED VIEW test3;');
+
+# an additional row to check if the REFRESH worked
+$node_publisher->safe_psql('postgres', q{INSERT INTO test2 (a, b) VALUES (30, 'thirty');});
+
+wait_for_caught_up($node_publisher, $appname);
+
+is($node_subscriber->safe_psql('postgres', q{SELECT COUNT(1) FROM test2}), 3,
+   'data replicated to subscriber after refresh');
+
 $node_subscriber->stop;
 $node_publisher->stop;
-- 
1.8.3.1