Hi,

On 2026-02-07 20:30:41 -0500, Robert Haas wrote:
> On Sat, Feb 7, 2026 at 5:47 PM Heikki Linnakangas <[email protected]> wrote:
> > The thing I like least about this is how the upgrade works, i.e. the
> > conversion code and the "double xmax" hack. This would look much nicer
> > if we could start from clean slate and just add the fields we need to
> > the page header. However, upgrade is important, that point has been
> > discussed a lot on the list, and I don't have any better ideas. I think
> > it's as good as it gets at the high level.
> 
> I don't think the page header is the right thing, because that applies
> to every AM, including both table AMs and index AMs. I'd say that some
> of the things we already have in the page header don't really make
> sense there -- in particular, pd_prune_xid, which is heap-specific. We
> can't change that at this point, but we shouldn't make it worse.

On the topic of pd_prune_xid - I've been wondering if, instead of the double
xmax approach, the high bits of the 64bit xid could be stored in pd_prune_xid,
signified by a flag on the page indicating so.



WRT heap specific stuff in non-heap code: I think the way that the patch
integrates the format conversion into bufmgr's IO path is pretty much
unacceptable (the call to convert_page() is in
buffer_readv_complete_one()). Completely obviously the bufmgr layer has no
business doing so, and there's absolutely no guarantee that a relkind =
r/t/... page is a heap page.

The patch also adds a pointer to the open relation to IO handles, to be able
to know what relkind a buffer being read into is. But that's nonsensical, the
backend actually executes the completion handler might not be the backend that
issued the IO, which means that that pointer can be completely bogus.  Also
the relation might be closed by that point (think of a query that errors out
after starting IO).  If this patch passes tests, I'm rather baffled.


I strongly agree with the points made upthread about not changing
TransactionId to 64bit, we already have a 64bit representation - what's the
point of having two and introducing a new ShortTransactionId type?


Greetings,

Andres Freund


Reply via email to