On Fri, Feb 21, 2025 at 5:00 PM Melanie Plageman
<melanieplageman@gmail.com> wrote:
>
> I thought about doing this, but in the end I decided against this
> approach. If we wanted to make it easy to palloc arrays of different
> sizes and have tbm_extract_page_tuple extract that many tuples into
> the array, we'd have to change more of the code anyway because
> tbm_extract_page_tuple is what tells us how many tuples there are to
> begin with. We could save this somewhere while filling in the
> PagetableEntries initially, but that is a bigger change.
>
> And, passing a length would also mean more callers would have to know
> about TBM_MAX_TUPLES_PER_PAGE, which I think is already kind of a hack
> since it is defined as MaxHeapTuplesPerPage.
I changed my mind. I think since the struct I added was only used for
tbm_extract_page_tuple(), it was a bit weird. I also think it is okay
for callers to use TBM_MAX_TUPLES_PER_PAGE. I ended up revising this
to use the same API you implemented for TIDStore in
TidStoreGetBlockOffsets(). The caller passes an array and the size of
the array and tbm_extract_page_tuple() fills in that many offsets and
returns the total number of offsets. It avoids adding a new struct and
means callers could pass a different value than
TBM_MAX_TUPLES_PER_PAGE. Overall, I like it.
- Melanie
