On Fri, Aug 4, 2017 at 5:50 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Michael Paquier <michael.paqu...@gmail.com> writes: >> So I think that the attached patch is able to do the legwork. > > I've pushed this into HEAD. It seems like enough of a behavioral > change that we wouldn't want to back-patch, but IMO it's not too late > to be making this type of change in v10. If we wait for the next CF > then it will take another year for the fix to reach the field.
Thanks for applying the fix. My intention when adding that in a CF is not to see things lost. >> While >> looking at the code, I have bumped into index_check_primary_key() that >> discarded the case of sending SET NOT NULL to child tables, as >> introduced by 88452d5b. But that's clearly an oversight IMO, and the >> comment is wrong to begin with because SET NOT NULL is spread to child >> tables. Using is_alter_table instead of a plain true in >> index_check_primary_key() for the call of AlterTableInternal() is >> defensive, but I don't think that we want to impact any modules >> relying on this API, so recursing only when ALTER TABLE is used is the >> safest course of action to me. > > I didn't find that persuasive: I think passing "recurse" as just plain > "true" is safer and more readable. We shouldn't be able to get there > in a CREATE case, as per the comments; and if we did, there shouldn't > be any child tables anyway; but if somehow there were, surely the same > consistency argument would imply that we *must* recurse and fix those > child tables. So I just made it "true". Yeah, that matches what I read as well. No complains to switch to a plain "true" even if it is not reached yet. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
