Robert Haas <robertmhaas@gmail.com> writes:
> I did not find this version very clear. It wasn't consistent about
> using ObjectIdGetDatum() where needed, but the bigger problem was that
> I found the logic unnecessarily convoluted. I rewrote it - I believe
> more straightforwardly - as attached. How does this look?
I'd suggest that we save some code by always doing separate updates for
the commutator and negator entries. We can handle the corner case where
they're the same by doing a CommandCounterIncrement between the updates,
instead of having convoluted and probably-never-yet-tested logic.
I'm also a bit dubious of the assumption in RemoveOperatorById that an
operator can't be its own negator. Yeah, that should not be the case,
but if it is the case the deletion will fail outright.
We could resolve both of these issues by changing the semantics of
OprUpdate so that it unconditionally does a CommandCounterIncrement
after each update that it performs. IMO that would be a lot simpler
and more bulletproof; it'd allow removal of a lot of these
overly-tightly-reasoned cases.
regards, tom lane
