Hi, On 2019-07-31 10:42:50 -0700, Ashwin Agrawal wrote: > On Tue, Jul 30, 2019 at 2:58 PM Thomas Munro <thomas.mu...@gmail.com> wrote: > > > On Tue, Jun 25, 2019 at 6:02 AM Andres Freund <and...@anarazel.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
