Re: Rename of triggers for partitioned tables

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> I pushed this patch with some minor corrections (mostly improved the
> message wording.)

Coverity does not like this bit, and I fully agree:

/srv/coverity/git/pgsql-git/postgresql/src/backend/commands/trigger.c: 1639 in renametrig_partition()
>>>     CID 1489387:  Incorrect expression  (ASSERT_SIDE_EFFECT)
>>>     Argument "found++" of Assert() has a side effect.  The containing function might work differently in a
non-debugbuild. 
1639             Assert(found++ <= 0);

Perhaps there's no actual bug there, but it's still horrible coding.
For one thing, the implication that found could be negative is extremely
confusing to readers.  A boolean might be better.  However, I wonder why
you bothered with a flag in the first place.  The usual convention if
we know there can be only one match is to just not write a loop at all,
with a suitable comment, like this pre-existing example elsewhere in
trigger.c:

        /* There should be at most one matching tuple */
        if (HeapTupleIsValid(tuple = systable_getnext(tgscan)))

If you're not quite convinced there can be only one match, then it
still shouldn't be an Assert --- a real test-and-elog would be better.

            regards, tom lane