Re: REL_12_STABLE crashing with assertion failure in ExtractReplicaIdentity

Amit Langote <amitlangote09(at)gmail(dot)com> writes:
> On Mon, Sep 2, 2019 at 6:31 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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