On Wed, Dec 13, 2017 at 10:41 AM, Amit Langote <langote_amit...@lab.ntt.co.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_kyxyy1...@mail.gmail.com Or this one: http://www.postgresql.org/message-id/21633.1448383...@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
