On 8/26/20, 2:13 PM, "Bossart, Nathan" <bossartn@amazon.com> wrote:
> On 8/26/20, 12:16 PM, "Alvaro Herrera" <alvherre@2ndquadrant.com> wrote:
>> On 2020-Aug-20, Jeremy Schneider wrote:
>>> Alternatively, if we don't want to take this approach, then I'd argue
>>> that we should update README.tuplock to explicitly state that
>>> XMAX_LOCK_ONLY and XMAX_COMMITTED are incompatible (just as it already
>>> states for HEAP_XMAX_IS_MULTI and HEAP_XMAX_COMMITTED) and clean up the
>>> code in heapam_visibility.c for consistency.
>>
>> Yeah, I like this approach better for the master branch; not just clean
>> up as in remove the cases that handle it, but also actively elog(ERROR)
>> if the condition ever occurs (hopefully with some known way to fix the
>> problem; maybe by "WITH tup AS (DELETE FROM tab WHERE .. RETURNING *)
>> INSERT * INTO tab FROM tup" or similar.)
>
> +1. I wouldn't mind picking this up, but it might be some time before
> I can get to it.
I've finally gotten started on this, and I've attached a work-in-
progress patch to gather some early feedback. I'm specifically
wondering if there are other places it'd be good to check for these
unsupported combinations and whether we should use the
HEAP_XMAX_IS_LOCKED_ONLY macro or just look for the
HEAP_XMAX_LOCK_ONLY bit. IIUC HEAP_XMAX_IS_LOCKED_ONLY is intended to
handle some edge cases after pg_upgrade, but AFAICT
HEAP_XMAX_COMMITTED was not used for those previous bit combinations,
either. Therefore, I've used the HEAP_XMAX_IS_LOCKED_ONLY macro in
the attached patch, but I would not be surprised to learn that this is
wrong for some reason.
Nathan