Re: Remove HeapTuple and Buffer dependency for predicate locking functions

Hi,

On 2019-07-31 10:42:50 -0700, Ashwin Agrawal wrote:
> On Tue, Jul 30, 2019 at 2:58 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
>
> > On Tue, Jun 25, 2019 at 6:02 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
> > > > - CheckForSerializableConflictOut() no more takes HeapTuple nor
> > > > buffer, instead just takes xid. Push heap specific parts from
> > > > CheckForSerializableConflictOut() into its own function
> > > > HeapCheckForSerializableConflictOut() which calls
> > > > CheckForSerializableConflictOut(). The alternative option could be
> > > > CheckForSerializableConflictOut() take callback function and
> > > > callback arguments, which gets called if required after performing
> > > > prechecks. Though currently I fell AM having its own wrapper to
> > > > perform AM specific task and then calling
> > > > CheckForSerializableConflictOut() is fine.
> > >
> > > I think it's right to move the xid handling out of
> > > CheckForSerializableConflictOut(). But I think we also ought to move the
> > > subtransaction handling out of the function - e.g. zheap doesn't
> > > want/need that.
> >
> > Thoughts on this Ashwin?
> >
>
> I think the only part its doing for sub-transaction is
> SubTransGetTopmostTransaction(xid). If xid passed to this function is
> already top most transaction which is case for zheap and zedstore, then
> there is no downside to keeping that code here in common place.

Well, it's far from a cheap function. It'll do unnecessary on-disk
lookups in many cases. I'd call that quite a downside.

Greetings,

Andres Freund