|From:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>|
|To:||Michael Paquier <michael(dot)paquier(at)gmail(dot)com>|
|Cc:||tushar <tushar(dot)ahuja(at)enterprisedb(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL|
|Views:||Raw Message | Whole Thread | Download mbox|
Michael Paquier <michael(dot)paquier(at)gmail(dot)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.
> 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".
regards, tom lane
|Next Message||Masahiko Sawada||2017-08-04 16:02:55||Re: Subscription code improvements|
|Previous Message||Michael Paquier||2017-08-04 14:27:17||Re: Cache lookup errors with functions manipulation object addresses|