On Mon, Feb 7, 2022 at 9:42 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Mon, Feb 7, 2022 at 12:26 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > I have splitted the patch into multiple patches which can be
> > independently committable and easy to review. I have explained the
> > purpose and scope of each patch in the respective commit messages.
>
> Hmm. The parts of this I've looked at seem reasonably clean, but I
> don't think I like the design choice. You're inventing
> RelFileNodeSetFork(), but at present the RelFileNode struct doesn't
> include a fork number. I feel like we should leave that alone, and
> only change the definition of a BufferTag. What about adding accessors
> for all of the BufferTag fields in 0001, and then in 0002 change it to
> look like something this:
>
> typedef struct BufferTag
> {
> Oid dbOid;
> Oid tablespaceOid;
> uint32 fileNode_low;
> uint32 fileNode_hi:24;
> uint32 forkNumber:8;
> BlockNumber blockNumber;
> } BufferTag;
Okay, we can do that. But we can not leave RelFileNode untouched I
mean inside RelFileNode also we will have to change the relNode as 2
32 bit integers, I mean like below.
> typedef struct RelFileNode
> {
> Oid spcNode;
> Oid dbNode;
> uint32 relNode_low;
> uint32 relNode_hi;
} RelFileNode;
For RelFileNode also we need to use 2, 32-bit integers so that we do
not add extra alignment padding because there are a few more
structures that include RelFileNode e.g. xl_xact_relfilenodes,
RelFileNodeBackend, and many other structures.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
