Re: not null constraints, again

Tender Wang <tndrwang@gmail.com>

From: Tender Wang <tndrwang@gmail.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, jian he <jian.universality@gmail.com>, Pg Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-04-16T06:28:27Z
Lists: pgsql-hackers
Alvaro Herrera <alvherre@alvh.no-ip.org> 于2025年4月16日周三 03:12写道:

> On 2025-Apr-15, Tender Wang wrote:
>
> > I thought further about the lockmode calling find_inheritance_children
> > in ATPrepAddPrimaryKey.
> > What we do here? We first get oids of children, then check the if the
> > column of children has marked not-null,  if not, report an error.
> > No operation here on children.  I check other places that call
> > find_inheritance_children, if we have operation on children, we usually
> pass
> > Lockmode to find_inheritance_children, otherwise pass NoLock.
>
> Hmm, I'm wary of doing this, although you're perhaps right that there's
> no harm.  If we do need to add a not-null constraint on the children,
> surely we'll acquire a stronger lock further down the execution chain.
> In principle this sounds a good idea though.  (I'm not sure about doing
> SearchSysCacheAttName() on a relation that might be dropped
> concurrently; does dropping the child acquire lock on its parent?  I
> suppose so, in which case this is okay; but still icky.  What about
> DETACH CONCURRENTLY?)
>

Yes,  I'm also wary of doing this.
Although NoLock may fix this issue, I feel it will trigger other problems,
such as the scenario you listed above.


> However, I've also been looking at this and realized that this code can
> have different structure which may allows us to skip the
> find_inheritance_children() altogether.  The reason is that we already
> scan the parent's list of columns searching for not-null constraints on
> each of them; we only need to run this verification on children for
> columns where there is none in the parent, and then only in the case
> where recursion is turned off.


> So I propose the attached patch, which also has some comments to
> hopefully explain what is going on and why.  I ran Tom's test script a
> few hundred times in a loop and I see no deadlock anymore.
>

No objection from me.   The comments may need a little polishing.


-- 
Thanks,
Tender Wang

Commits

  1. Suppress "may be used uninitialized" warnings from older compilers.

  2. Elide not-null constraint checks on child tables during PK creation

  3. Remove unnecessary code to handle CONSTR_NOTNULL

  4. Silence compilers about extractNotNullColumn()

  5. Add pg_constraint rows for not-null constraints