On Thu, Oct 6, 2011 at 11:38 PM, Alex Hunsaker <bada...@gmail.com> wrote: >> Oh, I see the problem, and I now agree that it's the DROP CONSTRAINT >> code that is buggy. > > Want me to roll this fix in as part of the alter table only constraint > patch? Or keep it split out? We might want to backpatch to at least > 8.3 where HOT was introduced (yes looks like the bug existed back > then). I suppose its a fairly narrow chance to hit this bug so I could > see the argument for not back patching...
Yeah, I'm not inclined to back-patch it. The chance of hitting this in older versions must be very small, or somebody would have noticed by now. If we get a report from the field, we can always back-patch it then, but right now it doesn't seem worth taking any risks for. On a related note, your fix seems slightly fragile to me ... because we're pulling a CCI out of the innermost loop, but a CCI can still happen inside that same loop if we recurse, because the recursive call will do one before returning. Now, maybe that's OK, because the recursive call in that case will just be deleting the tuple, so there won't be a new version for us to stumble over. 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). 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. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
drop-constraint.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
