On Mon, Nov 16, 2015 at 4:27 AM, Marti Raudsepp <ma...@juffo.org> wrote: > Thank you so much for the review and patch update. I should have done that > myself, but I've been really busy for the last few weeks. :(
Maybe I'm having an attack of the stupids today, but it looks to me like the changes to pg_constraint.c look awfully strange to me. In the old code, if object_address_present() returns true, we continue, skipping the rest of the loop. In the new code, we instead set alreadyChanged to true. That causes both of the following if statements, as revised, to fall out, so that we skip the rest of the loop. Huh? Wouldn't a one line change to add oldNspId != newNspId to the criteria for a simple_heap_update be just as good? Backing up a bit, maybe we should be a bit more vigorous in treating a same-namespace move as a no-op. That is, don't worry about calling the post-alter hook in that case - just have AlterConstraintNamespaces start by checking whether oldNspId == newNspid right at the top; if so, return. The patch seems to have the idea that it is important to call the post-alter hook even in that case, but I'm not sure whether that's true. I'm not sure it's false, but I'm also not sure it's true. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
