On Wed, Nov 18, 2015 at 12:32 AM, Haribabu Kommi <kommi.harib...@gmail.com> wrote: > 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.
OK, committed with some additional cosmetic improvements. -- 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
