On 2022-Oct-21, Michael Paquier wrote:

> On Fri, Oct 21, 2022 at 05:53:25PM +0800, Richard Guo wrote:

> >     /* These conditions can not be both true */
> 
> If you do that, it would be a bit easier to read as of the following
> assertion instead?  Like:
> Assert(!during_backup_start ||
>        sessionBackupState == SESSION_BACKUP_NONE);

My intention here was that the Assert should be inside the block, that
is, we already know that at least one is true, and we want to make sure
that they are not *both* true.

AFAICT the attached patch also fixes the bug without making the assert
weaker.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
>From 710eef9259f4286f8d8660ac1dd898323205a037 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Sat, 22 Oct 2022 09:54:24 +0200
Subject: [PATCH] Fix misplaced assertion

---
 src/backend/access/transam/xlog.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index dea978a962..fb3fbcd2be 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8841,12 +8841,12 @@ do_pg_abort_backup(int code, Datum arg)
 {
 	bool		during_backup_start = DatumGetBool(arg);
 
-	/* Only one of these conditions can be true */
-	Assert(during_backup_start ^
-		   (sessionBackupState == SESSION_BACKUP_RUNNING));
-
 	if (during_backup_start || sessionBackupState != SESSION_BACKUP_NONE)
 	{
+		/* We should be here only by one of these reasons, never both */
+		Assert(during_backup_start ^
+			   (sessionBackupState == SESSION_BACKUP_RUNNING));
+
 		WALInsertLockAcquireExclusive();
 		Assert(XLogCtl->Insert.runningBackups > 0);
 		XLogCtl->Insert.runningBackups--;
-- 
2.30.2

Reply via email to