Amit Langote <amitlangot...@gmail.com> writes:
> On Mon, Sep 2, 2019 at 6:31 AM Tom Lane <t...@sss.pgh.pa.us> wrote:
>> Here's a proposed patch along those lines. It fixes Hadi's original
>> crash case and passes check-world.
> Agree that this patch would be a better solution for Hadi's report,
> although I also agree that the situation with index locking for DELETE
> isn't perfect.
Thanks for checking!
>> I'm a bit suspicious of the exclusion for idattrs being empty, but
>> if I remove that, some of the contrib/test_decoding test results
>> change. Anybody want to comment on that? If that's actually an
>> expected situation, why is there an elog(DEBUG) in that path?
> ISTM that the exclusion case may occur with the table's replica
> identity being REPLICA_IDENTITY_DEFAULT and there being no primary
> index defined, in which case nothing needs to get logged.
Looking more closely, the case is unreachable in the heap_update
path because key_changed will necessarily be false if the idattrs
set is empty. But it is reachable in heap_delete because that
just passes key_changed = constant true, whether or not there's
any defined replica identity. In view of that, I think
we should just remove the elog(DEBUG) ... and maybe add a comment
explaining this.
regards, tom lane
