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

On Wed, Dec 13, 2017 at 10:41 AM, Amit Langote
<Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> On 2017/12/13 9:22, Ali Akbar wrote:
>> For me, it's better to prevent that from happening. So, attempts to
>> DROP NOT NULL on the child must be rejected. The attached patch does
>> that.
>>
>> Unfortunately, pg_class has no "has_parent" attribute, so in this
>> patch, it hits pg_inherits everytime DROP NOT NULL is attempted.
>>
>> Notes:
>> - It looks like we could remove the parent partition checking above?
>> Because the new check already covers what it does
>
> I noticed that too.
>
> So, if we're going to prevent DROP NOT NULL on *all* child table (not just
> partitions), we don't need the code that now sits there to prevent that
> only for partitions.

It is not the first time that this topic shows up. See for example
this thread from myself's version of last year:
https://www.postgresql.org/message-id/CAB7nPqTPXgX9HiyhhtAgpW7jbA1iskMCSoqXPEEB_KYXYy1E1Q@mail.gmail.com
Or this one:
http://www.postgresql.org/message-id/21633.1448383428@sss.pgh.pa.us

And the conclusion is that we don't want to do what you are doing
here, and that it would be better to store NOT NULL constraints in a
way similar to CHECK constraints.

> How about: ".. if some parent has the same"
>
> + heap_close(parent, AccessShareLock);
>
> Maybe, we shouldn't be dropping the lock so soon.

Yes, such things usually need to be kept until the end of the
transaction, and usually you need to be careful about potential lock
upgrades that could cause deadlocks. This patch looks wrong for both
of those things.
--
Michael