On Fri, Oct 7, 2011 at 11:19 AM, Alex Hunsaker <badalex@gmail.com> wrote:
> On Fri, Oct 7, 2011 at 07:53, Robert Haas <robertmhaas@gmail.com> wrote:
>
>> The only way we could
>> trip up in that case is if there were two identically named
>> constraints. We'd have to visit the first tuple, update it, then
>> visit the second tuple, recurse (thus incrementing the command
>> counter), and then visit the updated version of the first tuple. And
>> that should be impossible, because we've got code to disallow multiple
>> constraints on the same relation with the same name (though no unique
>> index, for some reason).
>
> Surely an oversight...
>
>> Still, that's a long chain of reasoning, so
>> I'm wondering if we can't come up with something that is more
>> obviously correct.
>>
>> If we're confident that the inner loop here should never iterate more
>> than once (i.e. the lack of a unique index is not an ominous sign)
>> then maybe we should just rewrite this so that the inner loop scans
>> until it finds a match and then terminates. Then, outside the loop,
>> we check whether a tuple was found and if so process it - but without
>> ever going back to look for another one. See attached.
>
> I eyeballed it and it does indeed seem simpler. My only thought is
> perhaps we should add that missing unique index on (conrelid,
> conname). If we are not going to support duplicate names in the code,
> we might as well enforce it. No?
Not sure. There could be performance or other ramifications to that.
For now I'm more interested in fixing this particular bug than I am in
getting into a wider world of re-engineering...
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
