Re: REL_12_STABLE crashing with assertion failure in ExtractReplicaIdentity

Amit Langote <amitlangote09@gmail.com> writes:
> On Mon, Sep 2, 2019 at 6:31 AM Tom Lane <tgl@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