Horton

Re: heapgettup refactoring

David Rowley <dgrowleyml@gmail.com>

View thread View on postgr.es
From: David Rowley <dgrowleyml@gmail.com>
To: Melanie Plageman <melanieplageman@gmail.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Andres Freund <andres@anarazel.de>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2023-02-07T04:40:13Z
Lists: pgsql-hackers

Attachments

On Fri, 3 Feb 2023 at 15:26, David Rowley <dgrowleyml@gmail.com> wrote:
> I've pushed all but the final 2 patches now.

I just pushed the final patch in the series.  I held back on moving
the setting of rs_inited back into the heapgettup_initial_block()
helper function as I wondered if we should even keep that field.

It seems that rs_cblock == InvalidBlockNumber in all cases where
rs_inited == false, so maybe it's better just to use that as a
condition to check if the scan has started or not. I've attached a
patch which does that.

I ended up adjusting HeapScanDescData more than what is minimally
required to remove rs_inited. I wondered if rs_cindex should be closer
to rs_cblock in the struct so that we're more likely to be adjusting
the same cache line than if that field were closer to the end of the
struct.  We don't need rs_coffset and rs_cindex at the same time, so I
made it a union. I see that the struct is still 712 bytes before and
after this change. I've not yet tested to see if there are any
performance gains to this change.

David

Commits

  1. Remove stray duplicated comment in heapam.h

  2. More refactoring of heapgettup() and heapgettup_pagemode()

  3. Run pgindent on heapam.c

  4. Push lpp variable closer to usage in heapgetpage()

  5. Variable renaming in preparation for refactoring

  6. Turn HeapKeyTest macro into inline function

  7. Remove unused include

  8. Remove redundant breaks in HeapTupleSatisfiesVisibility