Re: Remembering bug #6123

"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> ... All we have to do is start treating
>> HeapTupleSelfUpdated result from heap_update or heap_delete as an
>> error case instead of an okay-do-nothing case. There doesn't even
>> need to be an explicit check that this was caused by a trigger,
>> because AFAICS there isn't any other possibility.

> I think that's pretty much what my previously posted patches did.
> Here's a slightly modified one, based on Florian's feedback. Is
> this what you had in mind, or am I misunderstanding?

Actually, I was just about to follow up with a comment that that was too
simplistic: it would break the current behavior of join updates/deletes
that join to the same target row more than once. (There might be an
argument for restricting those, but this discussion isn't about that.)
So what we need to do is check whether the outdate was done by a later
CommandId than current. I see that your patch is attempting to deal
with these issues by testing GetCurrentCommandId(false) !=
estate->es_output_cid, but that seems completely wrong to me, as what it
does is complain if *any* additional command has been executed in the
transaction, regardless of what changed the target tuple. It ought to
be comparing the tuple's xmax to es_output_cid. And the comment needs
to cover why it's worrying about that.

Also, what's the point of testing update_ctid? I don't see that it
matters whether the outdate was a delete or an update.

regards, tom lane