Subject: Re: Can't find not null constraint, but \d+ shows that

From: Tender Wang <[email-masked]>
Date: Thu, 28 Mar 2024 20:05:04 +0800

Alvaro Herrera <[email-masked]> 于2024年3月28日周四 17:18写道：

> On 2024-Mar-28, Tender Wang wrote:
>
> >  RemoveConstraintById() should think recurse(e.g. partition table)? I'm
> not
> > sure now.
> >  If we should think process recurse in RemoveConstraintById(),  the
> > function will look complicated than before.
>
> No -- this function handles just a single constraint, as identified by
> OID.  The recursion is handled by upper layers, which can be either
> dependency.c or tablecmds.c.  I think the problem you found is caused by
> the fact that I worked with the tablecmds.c recursion and neglected the
> one in dependency.c.
>

Indeed.

create table skip_wal_skip_rewrite_index (c varchar(10) primary key);
alter table skip_wal_skip_rewrite_index alter c type varchar(20);

Above SQL need attnotnull to be true when re-add index, but
RemoveConstraintById() is hard to recognize this scenario as I know.
We should re-set attnotnull to be true before re-add index. I add a new
AT_PASS in attached patch.
Any thoughts?
--
Tender Wang
OpenPie:  https://en.openpie.com/

---

From: jian he <[email-masked]>
Date: Fri, 29 Mar 2024 14:56:47 +0800

hi.
about v4, i think, i understand the changes you made.
RemoveConstraintById(Oid conId)
will drop a single constraint record.
if the constraint is primary key, then primary key associated
attnotnull should set to false.
but sometimes it shouldn't.

for example:
drop table if exists t2;
CREATE TABLE t2(c0 int, c1 int);
ALTER TABLE  t2 ADD CONSTRAINT t2_pk PRIMARY KEY(c0, c1);
ALTER TABLE t2 ALTER COLUMN c0 ADD GENERATED ALWAYS AS IDENTITY;
ALTER TABLE  t2 DROP c1;

+ * If this was a NOT NULL or the primary key, the constrained columns must
+ * have had pg_attribute.attnotnull set.  See if we need to reset it, and
+ * do so.
+ */
+ if (unconstrained_cols != NIL)

unconstrained_cols is not NIL, which means we have dropped a primary key.
I am confused by "If this was a NOT NULL or the primary key".

+
+ /*
+ * Since the above deletion has been made visible, we can now
+ * search for any remaining constraints on this column (or these
+ * columns, in the case we're dropping a multicol primary key.)
+ * Then, verify whether any further NOT NULL exists, and reset
+ * attnotnull if none.
+ */
+ contup = findNotNullConstraintAttnum(RelationGetRelid(rel), attnum);
+ if (HeapTupleIsValid(contup))
+ {
+ heap_freetuple(contup);
+ heap_freetuple(atttup);
+ continue;
+ }

I am a little bit confused by the above comment.
I think the comments should say,
if contup is valid, that means, we already have one  "not null"
constraint associate with the attnum
in that condition, we must not set attnotnull, otherwise the
consistency between attnotnull and "not null"
table constraint will be broken.

other than that, the change in RemoveConstraintById looks sane.