On Wed, Nov 18, 2015 at 6:02 AM, Robert Haas <robertmh...@gmail.com> wrote: > 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?
Yes, that's correct, the above change can be written as you suggested. Updated patch attached with correction. > 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. I am also not sure whether calling the post-alter hook in case of constraint is necessarily required? but it was doing for other objects, so I suggested that way. Regards, Hari Babu Fujitsu Australia
Description: Binary data
-- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers