On Mon, Nov 16, 2015 at 4:27 AM, Marti Raudsepp <marti@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