Horton
Menu

Re: pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL

Michael Paquier <michael.paquier@gmail.com>

View thread
From: Michael Paquier <michael.paquier@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: tushar <tushar.ahuja@enterprisedb.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2017-08-04T16:06:38Z
Lists: pgsql-hackers 
On Fri, Aug 4, 2017 at 5:50 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Michael Paquier <michael.paquier@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

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix new pg_upgrade query not to rely on regnamespace

  2. pg_upgrade: check for inconsistencies in not-null constraints w/inheritance

  3. Apply ALTER ... SET NOT NULL recursively in ALTER ... ADD PRIMARY KEY.