Use bool with synced field (src/include/replication/slot.h)

Started by Ranier Vilela4 months ago6 messages
#1Ranier Vilela
ranier.vf@gmail.com
1 attachment(s)

Hi.

IMO in the struct ReplicationSlotPersistentData, the field
*synced* has wrong type declaration.
The intention was *bool* not *char* type.

Since it is only used in Boolean comparison.

trivial patch attached.

best regards,
Ranier Vilela

Attachments:

v1-001-use-bool-for-synced-field-slot.patchapplication/octet-stream; name=v1-001-use-bool-for-synced-field-slot.patchDownload
diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h
index e8fc342d1a..fe62162cde 100644
--- a/src/include/replication/slot.h
+++ b/src/include/replication/slot.h
@@ -134,7 +134,7 @@ typedef struct ReplicationSlotPersistentData
 	/*
 	 * Was this slot synchronized from the primary server?
 	 */
-	char		synced;
+	bool		synced;
 
 	/*
 	 * Is this a failover slot (sync candidate for standbys)? Only relevant
#2Nathan Bossart
nathandbossart@gmail.com
In reply to: Ranier Vilela (#1)
Re: Use bool with synced field (src/include/replication/slot.h)

On Tue, Sep 02, 2025 at 09:47:49AM -0300, Ranier Vilela wrote:

IMO in the struct ReplicationSlotPersistentData, the field *synced* has
wrong type declaration. The intention was *bool* not *char* type.

Since it is only used in Boolean comparison.

LGTM. In the original thread, it looks like this was added in v32 [0]/messages/by-id/CAJpy0uC2gYybBcvYCW68wvR-3akWnFbr7x66nZg7XTyjELW9iw@mail.gmail.com and
effectively switched to a bool in v58 [1]/messages/by-id/OS0PR01MB57169DD55EC8D9D1EDB7A0C2946A2@OS0PR01MB5716.jpnprd01.prod.outlook.com.

[0]: /messages/by-id/CAJpy0uC2gYybBcvYCW68wvR-3akWnFbr7x66nZg7XTyjELW9iw@mail.gmail.com
[1]: /messages/by-id/OS0PR01MB57169DD55EC8D9D1EDB7A0C2946A2@OS0PR01MB5716.jpnprd01.prod.outlook.com

--
nathan

#3Nathan Bossart
nathandbossart@gmail.com
In reply to: Nathan Bossart (#2)
Re: Use bool with synced field (src/include/replication/slot.h)

On Tue, Sep 02, 2025 at 11:02:37AM -0500, Nathan Bossart wrote:

LGTM. In the original thread, it looks like this was added in v32 [0] and
effectively switched to a bool in v58 [1].

I was worried that we might need to bump SLOT_VERSION for this, but I see
that as of v18 [0]https://postgr.es/c/97525bc5c8, we require sizeof(bool) == 1. So I think it can be
applied as-is, which I'm planning to do shortly.

[0]: https://postgr.es/c/97525bc5c8

--
nathan

#4Nathan Bossart
nathandbossart@gmail.com
In reply to: Nathan Bossart (#3)
Re: Use bool with synced field (src/include/replication/slot.h)

Committed.

--
nathan

#5Nathan Bossart
nathandbossart@gmail.com
In reply to: Nathan Bossart (#4)
Re: Use bool with synced field (src/include/replication/slot.h)

On Tue, Sep 02, 2025 at 04:54:59PM -0500, Nathan Bossart wrote:

Committed.

Gah, the commit message lists your name under the Discussion tag, and I
missed adding the link to the archives. Please forgive the brain fade.

--
nathan

#6Ranier Vilela
ranier.vf@gmail.com
In reply to: Nathan Bossart (#5)
Re: Use bool with synced field (src/include/replication/slot.h)

Em ter., 2 de set. de 2025 às 18:57, Nathan Bossart <
nathandbossart@gmail.com> escreveu:

On Tue, Sep 02, 2025 at 04:54:59PM -0500, Nathan Bossart wrote:

Committed.

Gah, the commit message lists your name under the Discussion tag, and I
missed adding the link to the archives. Please forgive the brain fade.

Don't worry, thanks for the commit.

best regards,
Ranier Vilela