Re: Fix comments in gistxlogDelete, xl_heap_freeze_page and xl_btree_delete

2023-03-02 Thread Drouvot, Bertrand

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.gmail.com

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.comFrom 075a8d1c3dbc1ef339c35f719faef758f855614e Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot 
Date: Thu, 2 Mar 2023 12:46:01 +
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



Re: Fix comments in gistxlogDelete, xl_heap_freeze_page and xl_btree_delete

2023-03-03 Thread Amit Kapila
On Thu, Mar 2, 2023 at 6:35 PM Drouvot, Bertrand
 wrote:
>
> 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.gmail.com
> >
> > 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.
>

LGTM.

-- 
With Regards,
Amit Kapila.




Re: Fix comments in gistxlogDelete, xl_heap_freeze_page and xl_btree_delete

2023-03-03 Thread Drouvot, Bertrand

Hi,

On 3/3/23 12:30 PM, Amit Kapila wrote:

On Thu, Mar 2, 2023 at 6:35 PM Drouvot, Bertrand
 wrote:


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.gmail.com

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.



LGTM.



Thanks for having looked at it!

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com




Re: Fix comments in gistxlogDelete, xl_heap_freeze_page and xl_btree_delete

2023-03-03 Thread Robert Haas
On Fri, Mar 3, 2023 at 11:28 AM Drouvot, Bertrand
 wrote:
> Thanks for having looked at it!

+1. Committed.

-- 
Robert Haas
EDB: http://www.enterprisedb.com




Re: Fix comments in gistxlogDelete, xl_heap_freeze_page and xl_btree_delete

2023-03-04 Thread Drouvot, Bertrand

Hi,

On 3/3/23 6:53 PM, Robert Haas wrote:

On Fri, Mar 3, 2023 at 11:28 AM Drouvot, Bertrand
 wrote:

Thanks for having looked at it!


+1. Committed.



Thanks!

Not a big deal, but the commit message that has been used is not 100% accurate.

Indeed, for gistxlogDelete, that's the other way around (as
compare to what the commit message says).

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com




Re: Fix comments in gistxlogDelete, xl_heap_freeze_page and xl_btree_delete

2023-03-06 Thread Robert Haas
On Sat, Mar 4, 2023 at 3:33 AM Drouvot, Bertrand
 wrote:
> Indeed, for gistxlogDelete, that's the other way around (as
> compare to what the commit message says).

Woops. Good point.

-- 
Robert Haas
EDB: http://www.enterprisedb.com