From: Euler Taveira <euler@eulerto.com>
To: pgsql-hackers@lists.postgresql.org
Subject: Ignore heap rewrites for materialized views in logical replication
Date: Fri, 27 May 2022 18:12:54 -0300
Message-ID: <bc557ebe-92dc-4afa-b6bb-285a9eeaa614@www.fastmail.com> (raw)
Hi,
While investigating an internal report, I concluded that it is a bug. The
reproducible test case is simple (check 0002) and it consists of a FOR ALL
TABLES publication and a non-empty materialized view on publisher. After the
setup, if you refresh the MV, you got the following message on the subscriber:
ERROR: logical replication target relation "public.pg_temp_NNNNN" does not exist
That's because the commit 1a499c2520 (that fixes the heap rewrite for tables)
forgot to consider that materialized views can also create transient heaps and
they should also be skipped. The affected version is only 10 because 11
contains a different solution (commit 325f2ec555) that provides a proper fix
for the heap rewrite handling in logical decoding.
0001 is a patch to skip MV too. I attached 0002 to demonstrate the issue but it
doesn't seem appropriate to be included. The test was written to detect the
error and bail out. After this fix, it takes a considerable amount of time to
finish the test because it waits for a message that never arrives. Since nobody
reports this bug in 5 years and considering that version 10 will be EOL in 6
months, I don't think an additional test is crucial here.
--
Euler Taveira
EDB https://www.enterprisedb.com/Attachments:download all (2 files, .tar.gz)v1-0001-Ignore-heap-rewrites-for-materialized-views-in-lo.patch (text/x-patch, 1.6K) download
From 652efe45665d91f2f4ae865dba078fcaffdc0a17 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 v1 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.cindex 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
v1-0002-test-heap-rewrite-for-materialized-view-in-logica.patch (text/x-patch, 2.0K) download
From 7ba28dea6800359ae631c1a132f8f6c6d418548b 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 v1 2/2] test: heap rewrite for materialized view in logical
replication
---
src/test/subscription/t/006_rewrite.pl | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/src/test/subscription/t/006_rewrite.pl b/src/test/subscription/t/006_rewrite.plindex 5e3211aefa..e5e0fdabe7 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,21 @@ 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;');++my $max_attempts = 2 * $TestLib::timeout_default;+my $attempts = 0;+my $errmsg = qr/logical replication target relation.*does not exist/;+{+ while ($attempts < $max_attempts)+ {+ last if (slurp_file($node_subscriber->logfile) =~ $errmsg);++ usleep(100_000);+ $attempts++;+ }+}+is($attempts, $max_attempts, 'heap rewrite for materialized view on subscriber');+
$node_subscriber->stop;
$node_publisher->stop;
--
2.30.2
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: pgsql-hackers@postgresql.org
Cc: euler@eulerto.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Ignore heap rewrites for materialized views in logical replication
In-Reply-To: <bc557ebe-92dc-4afa-b6bb-285a9eeaa614@www.fastmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox