public inbox for [email protected]  
help / color / mirror / Atom feed
From: Drouvot, Bertrand <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Cc: Robert Haas <[email protected]>
Subject: Re: Fix comments in gistxlogDelete, xl_heap_freeze_page and xl_btree_delete
Date: Thu, 2 Mar 2023 14:04:58 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>

Hi,

On 1/6/23 11:05 AM, Drouvot, Bertrand wrote:
> Hi hackers,
> 
> Please find attached a patch to $SUBJECT.
> 
> The wrong comments have been discovered by Robert in [1].
> 
> Submitting this here as a separate thread so it does not get lost in the logical decoding
> on standby thread.
> 
> [1]: https://www.postgresql.org/message-id/CA%2BTgmoYTTsxP8y6uknZvCBNCRq%2B1FJ4zGbX8Px1TGW459fGsaQ%40mail...
> 
> Looking forward to your feedback,
> 
> Regards,
> 

It looks like I did not create a CF entry for this one: fixed with [1].

Also, while at it, adding a commit message in V2 attached.

[1]: https://commitfest.postgresql.org/43/4235/

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
From 075a8d1c3dbc1ef339c35f719faef758f855614e Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Thu, 2 Mar 2023 12:46:01 +0000
Subject: [PATCH v2] Fix comments in gistxlogDelete, xl_heap_freeze_page and
 xl_btree_delete

gistxlogDelete claims that OffsetNumbers are in payload of blk 0, which is not
as they follow the main payload.

xl_heap_freeze_page claims that freeze plans and offset numbers follow, but
they don't: they're in the data for block 0.

xl_btree_delete claims that the data follows but they are attached to block 0.

This commit fixes the related comments.
---
 src/include/access/gistxlog.h    | 4 +---
 src/include/access/heapam_xlog.h | 5 +++--
 src/include/access/nbtxlog.h     | 9 ++++++---
 3 files changed, 10 insertions(+), 8 deletions(-)
 100.0% src/include/access/

diff --git a/src/include/access/gistxlog.h b/src/include/access/gistxlog.h
index 09f9b0f8c6..2ce9366277 100644
--- a/src/include/access/gistxlog.h
+++ b/src/include/access/gistxlog.h
@@ -52,9 +52,7 @@ typedef struct gistxlogDelete
 	TransactionId snapshotConflictHorizon;
 	uint16		ntodelete;		/* number of deleted offsets */
 
-	/*
-	 * In payload of blk 0 : todelete OffsetNumbers
-	 */
+	/* TODELETE OFFSET NUMBER ARRAY FOLLOWS */
 } gistxlogDelete;
 
 #define SizeOfGistxlogDelete	(offsetof(gistxlogDelete, ntodelete) + sizeof(uint16))
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index 8cb0d8da19..a2c67d1cd3 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -345,8 +345,9 @@ typedef struct xl_heap_freeze_page
 	TransactionId snapshotConflictHorizon;
 	uint16		nplans;
 
-	/* FREEZE PLANS FOLLOW */
-	/* OFFSET NUMBER ARRAY FOLLOWS */
+	/*
+	 * In payload of blk 0 : FREEZE PLANS and OFFSET NUMBER ARRAY
+	 */
 } xl_heap_freeze_page;
 
 #define SizeOfHeapFreezePage (offsetof(xl_heap_freeze_page, nplans) + sizeof(uint16))
diff --git a/src/include/access/nbtxlog.h b/src/include/access/nbtxlog.h
index edd1333d9b..e7a9711767 100644
--- a/src/include/access/nbtxlog.h
+++ b/src/include/access/nbtxlog.h
@@ -236,9 +236,12 @@ typedef struct xl_btree_delete
 	uint16		ndeleted;
 	uint16		nupdated;
 
-	/* DELETED TARGET OFFSET NUMBERS FOLLOW */
-	/* UPDATED TARGET OFFSET NUMBERS FOLLOW */
-	/* UPDATED TUPLES METADATA (xl_btree_update) ARRAY FOLLOWS */
+	/*
+	 * In payload of blk 0 :
+	 * - DELETED TARGET OFFSET NUMBERS
+	 * - UPDATED TARGET OFFSET NUMBERS
+	 * - UPDATED TUPLES METADATA (xl_btree_update) ARRAY
+	 */
 } xl_btree_delete;
 
 #define SizeOfBtreeDelete	(offsetof(xl_btree_delete, nupdated) + sizeof(uint16))
-- 
2.34.1



Attachments:

  [text/plain] v2-0001-Fix-comments-in-gistxlogDelete-xl_heap_freeze_pag.patch (2.6K, ../[email protected]/2-v2-0001-Fix-comments-in-gistxlogDelete-xl_heap_freeze_pag.patch)
  download | inline diff:
From 075a8d1c3dbc1ef339c35f719faef758f855614e Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Thu, 2 Mar 2023 12:46:01 +0000
Subject: [PATCH v2] Fix comments in gistxlogDelete, xl_heap_freeze_page and
 xl_btree_delete

gistxlogDelete claims that OffsetNumbers are in payload of blk 0, which is not
as they follow the main payload.

xl_heap_freeze_page claims that freeze plans and offset numbers follow, but
they don't: they're in the data for block 0.

xl_btree_delete claims that the data follows but they are attached to block 0.

This commit fixes the related comments.
---
 src/include/access/gistxlog.h    | 4 +---
 src/include/access/heapam_xlog.h | 5 +++--
 src/include/access/nbtxlog.h     | 9 ++++++---
 3 files changed, 10 insertions(+), 8 deletions(-)
 100.0% src/include/access/

diff --git a/src/include/access/gistxlog.h b/src/include/access/gistxlog.h
index 09f9b0f8c6..2ce9366277 100644
--- a/src/include/access/gistxlog.h
+++ b/src/include/access/gistxlog.h
@@ -52,9 +52,7 @@ typedef struct gistxlogDelete
 	TransactionId snapshotConflictHorizon;
 	uint16		ntodelete;		/* number of deleted offsets */
 
-	/*
-	 * In payload of blk 0 : todelete OffsetNumbers
-	 */
+	/* TODELETE OFFSET NUMBER ARRAY FOLLOWS */
 } gistxlogDelete;
 
 #define SizeOfGistxlogDelete	(offsetof(gistxlogDelete, ntodelete) + sizeof(uint16))
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index 8cb0d8da19..a2c67d1cd3 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -345,8 +345,9 @@ typedef struct xl_heap_freeze_page
 	TransactionId snapshotConflictHorizon;
 	uint16		nplans;
 
-	/* FREEZE PLANS FOLLOW */
-	/* OFFSET NUMBER ARRAY FOLLOWS */
+	/*
+	 * In payload of blk 0 : FREEZE PLANS and OFFSET NUMBER ARRAY
+	 */
 } xl_heap_freeze_page;
 
 #define SizeOfHeapFreezePage (offsetof(xl_heap_freeze_page, nplans) + sizeof(uint16))
diff --git a/src/include/access/nbtxlog.h b/src/include/access/nbtxlog.h
index edd1333d9b..e7a9711767 100644
--- a/src/include/access/nbtxlog.h
+++ b/src/include/access/nbtxlog.h
@@ -236,9 +236,12 @@ typedef struct xl_btree_delete
 	uint16		ndeleted;
 	uint16		nupdated;
 
-	/* DELETED TARGET OFFSET NUMBERS FOLLOW */
-	/* UPDATED TARGET OFFSET NUMBERS FOLLOW */
-	/* UPDATED TUPLES METADATA (xl_btree_update) ARRAY FOLLOWS */
+	/*
+	 * In payload of blk 0 :
+	 * - DELETED TARGET OFFSET NUMBERS
+	 * - UPDATED TARGET OFFSET NUMBERS
+	 * - UPDATED TUPLES METADATA (xl_btree_update) ARRAY
+	 */
 } xl_btree_delete;
 
 #define SizeOfBtreeDelete	(offsetof(xl_btree_delete, nupdated) + sizeof(uint16))
-- 
2.34.1



view thread (3+ messages)  latest in thread

reply

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: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Fix comments in gistxlogDelete, xl_heap_freeze_page and xl_btree_delete
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox