Date: Wed, 15 Feb 2023 22:41:20 -0800
From: Nathan Bossart <nathandbossart@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Dilip Kumar <dilipbalaut@gmail.com>,
	"Ryo Matsumura (Fujitsu)" <matsumura.ryo@fujitsu.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: DDL result is lost by CREATE DATABASE with WAL_LOG strategy
Message-ID: <20230216064120.GA2016097@nathanxps13>
References: <TYCPR01MB6868677E499C9AD5123084B5E8A39@TYCPR01MB6868.jpnprd01.prod.outlook.com>
 <20230216000659.GA1932550@nathanxps13>
 <CAFiTN-tEAaq6csi_6TB8sz=5ZV=XjhneUB0x5icfK8b3TLn+Xw@mail.gmail.com>
 <Y+2+n4FuE6K5/1CX@paquier.xyz>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="wac7ysb48OaltWcw"
Content-Disposition: inline
In-Reply-To: <Y+2+n4FuE6K5/1CX@paquier.xyz>
Archived-At: <https://www.postgresql.org/message-id/20230216064120.GA2016097%40nathanxps13>
Precedence: bulk


--wac7ysb48OaltWcw
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Thu, Feb 16, 2023 at 02:26:55PM +0900, Michael Paquier wrote:
> So, if I am understanding this stuff right, this issue can create data
> corruption once a DDL updates any pages of pg_class stored in a
> template database that gets copied by this routine.  In this case, the
> patch sent makes sure that any page copied will get written once a
> checkpoint kicks in.  Shouldn't we have at least a regression test for
> such a scenario?  The issue can happen when updating a template
> database after creating a database from it, which is less worrying
> than the initial impression I got, still I'd like to think that we
> should have some coverage as of the special logic this code path
> relies on for pg_class when reading its buffers.

I was able to quickly hack together a TAP test that seems to reliably fail
before the fix and pass afterwards.  There's probably a better place for
it, though...

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

--wac7ysb48OaltWcw
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="create_db_fix_v2.patch"

diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index ef05633bb0..a0259cc593 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -296,7 +296,7 @@ ScanSourceDatabasePgClass(Oid tbid, Oid dbid, char *srcpath)
 		CHECK_FOR_INTERRUPTS();
 
 		buf = ReadBufferWithoutRelcache(rlocator, MAIN_FORKNUM, blkno,
-										RBM_NORMAL, bstrategy, false);
+										RBM_NORMAL, bstrategy, true);
 
 		LockBuffer(buf, BUFFER_LOCK_SHARE);
 		page = BufferGetPage(buf);
diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index 209118a639..6e9f8a7c7f 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -39,6 +39,7 @@ tests += {
       't/031_recovery_conflict.pl',
       't/032_relfilenode_reuse.pl',
       't/033_replay_tsp_drops.pl',
+      't/100_bugs.pl',
     ],
   },
 }
diff --git a/src/test/recovery/t/100_bugs.pl b/src/test/recovery/t/100_bugs.pl
new file mode 100644
index 0000000000..6429a352e9
--- /dev/null
+++ b/src/test/recovery/t/100_bugs.pl
@@ -0,0 +1,24 @@
+
+# Copyright (c) 2023, PostgreSQL Global Development Group
+
+# Tests for various bugs found over time
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# DDL on template is persisted after creating database using WAL_LOG strategy
+my $node = PostgreSQL::Test::Cluster->new('node');
+$node->init;
+$node->start;
+$node->safe_psql("postgres", "CREATE DATABASE test STRATEGY WAL_LOG;");
+$node->safe_psql("template1", "CREATE TABLE test (a INT);");
+$node->safe_psql("postgres", "CHECKPOINT;");
+$node->stop('immediate');
+$node->start;
+my $result = $node->safe_psql("template1", "SELECT count(*) FROM test;");
+is($result, "0", "check table still exists");
+$node->stop;
+
+done_testing();

--wac7ysb48OaltWcw--