Re: Remove HeapTuple and Buffer dependency for predicate locking functions

On Wed, Nov 13, 2019 at 7:27 PM Ashwin Agrawal <aagrawal(at)pivotal(dot)io> wrote:
> On Sun, Nov 10, 2019 at 8:21 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
>> but on another read-through of the main patch
>> I didn't like the comments for CheckForSerializableConflictOut() or
>> the fact that it checks SerializationNeededForRead() again, after I
>> thought a bit about what the contract for this API is now. Here's a
>> version with small fixup that I'd like to squash into the patch.
>> Please let me know what you think,
>
> The thought or reasoning behind having SerializationNeededForRead()
> inside CheckForSerializableConflictOut() is to keep that API clean and
> complete by itself. Only if AM like heap needs to perform some AM
> specific checking only for serialization needed case can do so but is
> not forced. So, if AM for example Zedstore doesn't need to do any
> specific checking, then it can directly call
> CheckForSerializableConflictOut(). With the modified fixup patch, the
> responsibility is unnecessarily forced to caller even if
> CheckForSerializableConflictOut() can do it. I understand the intent
> is to avoid duplicate check for heap.

OK, I kept only the small comment change from that little fixup patch,
and pushed this.

> I had proposed as alternative way in initial email and also later,
> didn't receive comment on that, so re-posting.

> typedef bool (*AMCheckForSerializableConflictOutCallback) (void *arg);
...
> Just due to void* callback argument aspect I didn't prefer that
> solution and felt AM performing checks and calling
> CheckForSerializableConflictOut() seems better. Please let me know
> how you feel about this.

Yeah. We could always come back to this idea if it looks better once
we have more experience with new table AMs.