On Sat, May 28, 2022, at 7:07 AM, Amit Kapila wrote:
> I agree with your analysis and the fix looks correct to me.
Thanks for checking.
> Instead of waiting for an error, we can try to insert into a new table
> created by the test case after the 'Refresh ..' command and wait for
> the change to be replicated by using wait_for_caught_up.
That's a good idea. [modifying the test...] I used the same table. Whenever the
new row arrives on the subscriber or it reads that error message, it bails out.
> Let's try to see if we can simplify the test so that it can be
> committed along with a fix. If we are not able to find any reasonable
> way then we can think of skipping it.
The new test is attached.
--
Euler Taveira
EDB https://www.enterprisedb.com/
From fe13dc44f210b5f14b97bf9a29a242c2211ba700 Mon Sep 17 00:00:00 2001
From: Euler Taveira <euler.tave...@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.tave...@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
