On Tue, Sep 15, 2020 at 9:09 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I wrote:
> >> (2) ALTER TABLE ONLY ... ADD CONSTRAINT on a partition root tries to get
> >> AccessExclusiveLock on all child partitions, despite the ONLY.
>
> > The cause of this seems to be that ATPrepSetNotNull is too dumb to
> > avoid recursing to all the child tables when the parent is already
> > attnotnull. Or is there a reason we have to recurse anyway?
>
> I wrote a quick patch for this part. It seems pretty safe and probably
> could be back-patched without fear.
The patch changes existing behavior in one case as shown below:
drop table if exists foo cascade;
create table foo (a int not null);
create table child () inherits (foo);
alter table child alter a drop not null;
alter table foo alter a set not null;
insert into child select null;
Currently, the last statement gives a "not null violated" error, but
no longer with the patch, because the alter statement before that now
finishes without setting NOT NULL in child.
The patch's theory that if the parent column has NOT NULL set then it
must be set in child tables too does not actually hold for plain
inheritance cases, because as shown above, NOT NULL can be dropped in
children independently of the parent. With partitioning, dropping NOT
NULL from child partitions is prevented:
drop table if exists bar cascade;
create table bar (a int) partition by list (a);
create table bar1 partition of bar (a not null) for values in (1);
alter table bar1 alter a drop not null;
ERROR: column "a" is marked NOT NULL in parent table
> (I also noticed that
> ATSimpleRecursion is being unnecessarily stupid: instead of the
> demonstrably not-future-proof relkind check, it could test relhassubclass,
> which is not only simpler and less likely to need future changes, but
> is able to save a scan of pg_inherits in a lot more cases.)
+1
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com