Re: Make relfile tombstone files conditional on WAL level

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