Thread: [PATCH] Refactor: Extract XLogRecord info

[PATCH] Refactor: Extract XLogRecord info

From
Xiaoran Wang
Date:
Hi,
I refactored the code of extracting XLogRecord info.
In XLogRecord, the high 4 bits in xl_info is used by rmgr.

typedef struct XLogRecord
{
    uint32      xl_tot_len;     /* total len of entire record */
    TransactionId xl_xid;       /* xact id */
    XLogRecPtr  xl_prev;        /* ptr to previous record in log */
    uint8       xl_info;        /* flag bits, see below */
    RmgrId      xl_rmid;        /* resource manager for this record */
    /* 2 bytes of padding here, initialize to zero */
    pg_crc32c   xl_crc;         /* CRC for this record */

    /* XLogRecordBlockHeaders and XLogRecordDataHeader follow, no padding */

} XLogRecord;

I found lots of the code to get the info as below

XLogRecGetInfo(record) & ~XLR_INFO_MASK

Actually, we can directly use XLR_RMGR_INFO_MASK(0xF0)
instead of XLR_INFO_MASK(0x0F), which is easier to understand.
Remove XLR_INFO_MASK as it is not used any more.


--
Best regards !
Xiaoran Wang
Attachment

Re: [PATCH] Refactor: Extract XLogRecord info

From
Steven Niu
Date:
Hi,

I like the idea of your change as it saves me out of converting-in-my-mind.

And I suggest to create a macro to do this job.
    #define getRmgrInfo(info) (info & XLR_RMGR_INFO_MASK)

Then the code can become:
XLogRecGetInfo(record) & ~XLR_INFO_MASK;
-->
getRmgrInfo(XLogRecGetInfo(record));

Thanks,
Steven


在 2025/6/9 14:23, Xiaoran Wang 写道:
> Hi,
> I refactored the code of extracting XLogRecord info.
> In XLogRecord, the high 4 bits in xl_info is used by rmgr.
> 
> typedef struct XLogRecord
> {
>      uint32      xl_tot_len;     /* total len of entire record */
>      TransactionId xl_xid;       /* xact id */
>      XLogRecPtr  xl_prev;        /* ptr to previous record in log */
>      uint8       xl_info;        /* flag bits, see below */
>      RmgrId      xl_rmid;        /* resource manager for this record */
>      /* 2 bytes of padding here, initialize to zero */
>      pg_crc32c   xl_crc;         /* CRC for this record */
> 
>      /* XLogRecordBlockHeaders and XLogRecordDataHeader follow, no 
> padding */
> 
> } XLogRecord;
> 
> I found lots of the code to get the info as below
> 
> XLogRecGetInfo(record) & ~XLR_INFO_MASK
> 
> Actually, we can directly use XLR_RMGR_INFO_MASK(0xF0)
> instead of XLR_INFO_MASK(0x0F), which is easier to understand.
> Remove XLR_INFO_MASK as it is not used any more.
> 
> 
> -- 
> Best regards !
> Xiaoran Wang




Re: [PATCH] Refactor: Extract XLogRecord info

From
wenhui qiu
Date:
HI 
> And I suggest to create a macro to do this job.
>        #define getRmgrInfo(info) (info & XLR_RMGR_INFO_MASK)
>
> Then the code can become:
> XLogRecGetInfo(record) & ~XLR_INFO_MASK;
> -->
> getRmgrInfo(XLogRecGetInfo(record))
+1 Agreed, this makes the code more readable.

Thanks

On Mon, Jun 9, 2025 at 2:46 PM Steven Niu <niushiji@gmail.com> wrote:
Hi,

I like the idea of your change as it saves me out of converting-in-my-mind.

And I suggest to create a macro to do this job.
        #define getRmgrInfo(info) (info & XLR_RMGR_INFO_MASK)

Then the code can become:
XLogRecGetInfo(record) & ~XLR_INFO_MASK;
-->
getRmgrInfo(XLogRecGetInfo(record));

Thanks,
Steven


在 2025/6/9 14:23, Xiaoran Wang 写道:
> Hi,
> I refactored the code of extracting XLogRecord info.
> In XLogRecord, the high 4 bits in xl_info is used by rmgr.
>
> typedef struct XLogRecord
> {
>      uint32      xl_tot_len;     /* total len of entire record */
>      TransactionId xl_xid;       /* xact id */
>      XLogRecPtr  xl_prev;        /* ptr to previous record in log */
>      uint8       xl_info;        /* flag bits, see below */
>      RmgrId      xl_rmid;        /* resource manager for this record */
>      /* 2 bytes of padding here, initialize to zero */
>      pg_crc32c   xl_crc;         /* CRC for this record */
>
>      /* XLogRecordBlockHeaders and XLogRecordDataHeader follow, no
> padding */
>
> } XLogRecord;
>
> I found lots of the code to get the info as below
>
> XLogRecGetInfo(record) & ~XLR_INFO_MASK
>
> Actually, we can directly use XLR_RMGR_INFO_MASK(0xF0)
> instead of XLR_INFO_MASK(0x0F), which is easier to understand.
> Remove XLR_INFO_MASK as it is not used any more.
>
>
> --
> Best regards !
> Xiaoran Wang



Re: [PATCH] Refactor: Extract XLogRecord info

From
Xiaoran Wang
Date:


Steven Niu <niushiji@gmail.com> 于2025年6月9日周一 14:46写道:
Hi,

I like the idea of your change as it saves me out of converting-in-my-mind.

And I suggest to create a macro to do this job.
        #define getRmgrInfo(info) (info & XLR_RMGR_INFO_MASK)

Then the code can become:
XLogRecGetInfo(record) & ~XLR_INFO_MASK;
-->
getRmgrInfo(XLogRecGetInfo(record));

Good idea, found lots of  'XLogRecGetInfo(record) & ~XLR_INFO_MASK;'
in the code. 

I add a macro XLogRecRmgrGetInfo(record) in patch 0002.

Thanks,
Steven


在 2025/6/9 14:23, Xiaoran Wang 写道:
> Hi,
> I refactored the code of extracting XLogRecord info.
> In XLogRecord, the high 4 bits in xl_info is used by rmgr.
>
> typedef struct XLogRecord
> {
>      uint32      xl_tot_len;     /* total len of entire record */
>      TransactionId xl_xid;       /* xact id */
>      XLogRecPtr  xl_prev;        /* ptr to previous record in log */
>      uint8       xl_info;        /* flag bits, see below */
>      RmgrId      xl_rmid;        /* resource manager for this record */
>      /* 2 bytes of padding here, initialize to zero */
>      pg_crc32c   xl_crc;         /* CRC for this record */
>
>      /* XLogRecordBlockHeaders and XLogRecordDataHeader follow, no
> padding */
>
> } XLogRecord;
>
> I found lots of the code to get the info as below
>
> XLogRecGetInfo(record) & ~XLR_INFO_MASK
>
> Actually, we can directly use XLR_RMGR_INFO_MASK(0xF0)
> instead of XLR_INFO_MASK(0x0F), which is easier to understand.
> Remove XLR_INFO_MASK as it is not used any more.
>
>
> --
> Best regards !
> Xiaoran Wang





--
Best regards !
Xiaoran Wang
Attachment

Re: [PATCH] Refactor: Extract XLogRecord info

From
Xiaoran Wang
Date:
Just upload all the patches together.

Xiaoran Wang <fanfuxiaoran@gmail.com> 于2025年6月9日周一 18:25写道:


Steven Niu <niushiji@gmail.com> 于2025年6月9日周一 14:46写道:
Hi,

I like the idea of your change as it saves me out of converting-in-my-mind.

And I suggest to create a macro to do this job.
        #define getRmgrInfo(info) (info & XLR_RMGR_INFO_MASK)

Then the code can become:
XLogRecGetInfo(record) & ~XLR_INFO_MASK;
-->
getRmgrInfo(XLogRecGetInfo(record));

Good idea, found lots of  'XLogRecGetInfo(record) & ~XLR_INFO_MASK;'
in the code. 

I add a macro XLogRecRmgrGetInfo(record) in patch 0002.

Thanks,
Steven


在 2025/6/9 14:23, Xiaoran Wang 写道:
> Hi,
> I refactored the code of extracting XLogRecord info.
> In XLogRecord, the high 4 bits in xl_info is used by rmgr.
>
> typedef struct XLogRecord
> {
>      uint32      xl_tot_len;     /* total len of entire record */
>      TransactionId xl_xid;       /* xact id */
>      XLogRecPtr  xl_prev;        /* ptr to previous record in log */
>      uint8       xl_info;        /* flag bits, see below */
>      RmgrId      xl_rmid;        /* resource manager for this record */
>      /* 2 bytes of padding here, initialize to zero */
>      pg_crc32c   xl_crc;         /* CRC for this record */
>
>      /* XLogRecordBlockHeaders and XLogRecordDataHeader follow, no
> padding */
>
> } XLogRecord;
>
> I found lots of the code to get the info as below
>
> XLogRecGetInfo(record) & ~XLR_INFO_MASK
>
> Actually, we can directly use XLR_RMGR_INFO_MASK(0xF0)
> instead of XLR_INFO_MASK(0x0F), which is easier to understand.
> Remove XLR_INFO_MASK as it is not used any more.
>
>
> --
> Best regards !
> Xiaoran Wang





--
Best regards !
Xiaoran Wang


--
Best regards !
Xiaoran Wang
Attachment

Re: [PATCH] Refactor: Extract XLogRecord info

From
Steven Niu
Date:
LGTM. I have no more comments. 

Regards,
Steven

Xiaoran Wang <fanfuxiaoran@gmail.com> 于2025年6月9日周一 18:31写道:
Just upload all the patches together.

Xiaoran Wang <fanfuxiaoran@gmail.com> 于2025年6月9日周一 18:25写道:


Steven Niu <niushiji@gmail.com> 于2025年6月9日周一 14:46写道:
Hi,

I like the idea of your change as it saves me out of converting-in-my-mind.

And I suggest to create a macro to do this job.
        #define getRmgrInfo(info) (info & XLR_RMGR_INFO_MASK)

Then the code can become:
XLogRecGetInfo(record) & ~XLR_INFO_MASK;
-->
getRmgrInfo(XLogRecGetInfo(record));

Good idea, found lots of  'XLogRecGetInfo(record) & ~XLR_INFO_MASK;'
in the code. 

I add a macro XLogRecRmgrGetInfo(record) in patch 0002.

Thanks,
Steven


在 2025/6/9 14:23, Xiaoran Wang 写道:
> Hi,
> I refactored the code of extracting XLogRecord info.
> In XLogRecord, the high 4 bits in xl_info is used by rmgr.
>
> typedef struct XLogRecord
> {
>      uint32      xl_tot_len;     /* total len of entire record */
>      TransactionId xl_xid;       /* xact id */
>      XLogRecPtr  xl_prev;        /* ptr to previous record in log */
>      uint8       xl_info;        /* flag bits, see below */
>      RmgrId      xl_rmid;        /* resource manager for this record */
>      /* 2 bytes of padding here, initialize to zero */
>      pg_crc32c   xl_crc;         /* CRC for this record */
>
>      /* XLogRecordBlockHeaders and XLogRecordDataHeader follow, no
> padding */
>
> } XLogRecord;
>
> I found lots of the code to get the info as below
>
> XLogRecGetInfo(record) & ~XLR_INFO_MASK
>
> Actually, we can directly use XLR_RMGR_INFO_MASK(0xF0)
> instead of XLR_INFO_MASK(0x0F), which is easier to understand.
> Remove XLR_INFO_MASK as it is not used any more.
>
>
> --
> Best regards !
> Xiaoran Wang





--
Best regards !
Xiaoran Wang


--
Best regards !
Xiaoran Wang

Re: [PATCH] Refactor: Extract XLogRecord info

From
Fabrízio de Royes Mello
Date:


On Mon, 9 Jun 2025 at 22:40 Steven Niu <niushiji@gmail.com> wrote:
LGTM. I have no more comments. 


The refactoring LGTM but do we really need two patches? IMHO you can just merge everything into a single patch.

Fabrízio de Royes Mello