Re: not null constraints, again

> > On Wed, Sep 11, 2024 at 2:18 AM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> > >
> > > Hello, here's a v2 of this patch. I have fixed --I think-- all the
> > > issues you and Tender Wang reported (unless I declined a fix in some
> > > previous email).
> > >

PREPARE constr_meta (name[]) AS
with cte as
(
select con.oid as conoid, conrelid::regclass, pc.relkind as relkind,
con.coninhcount as inhcnt
,con.conname, con.contype, con.connoinherit as noinherit
,con.conislocal as islocal, pa.attname, pa.attnum
from pg_constraint con join pg_class pc on pc.oid = con.conrelid
join pg_attribute pa on pa.attrelid = pc.oid
and pa.attnum = any(conkey)
where con.contype in ('n', 'p', 'c') and
pc.relname = ANY ($1)
)
select pg_get_constraintdef(conoid), * from cte
order by contype, inhcnt, islocal, attnum;

The above constr_meta is used to query meta info, nothing fancy.

---exampleA
drop table pp1,cc1, cc2;
create table pp1 (f1 int);
create table cc1 (f2 text, f3 int) inherits (pp1);
create table cc2(f4 float) inherits(pp1,cc1);
alter table pp1 alter column f1 set not null;
execute constr_meta('{pp1,cc1, cc2}');

---exampleB
drop table pp1,cc1, cc2;
create table pp1 (f1 int not null);
create table cc1 (f2 text, f3 int) inherits (pp1);
create table cc2(f4 float) inherits(pp1,cc1);
execute constr_meta('{pp1,cc1, cc2}');

Should exampleA and exampleB
return same pg_constraint->coninhcount
for not-null constraint "cc2_f1_not_null"
?

We only have this Synopsis
ALTER [ COLUMN ] column_name { SET | DROP } NOT NULL

--tests from src/test/regress/sql/inherit.sql
CREATE TABLE inh_nn_parent (a int, NOT NULL a NO INHERIT);
ALTER TABLE inh_nn_parent ALTER a SET NOT NULL;
current fail at ATExecSetNotNull
ERROR: cannot change NO INHERIT status of NOT NULL constraint
"inh_nn_parent_a_not_null" on relation "inh_nn_parent"

seems we translate
ALTER TABLE inh_nn_parent ALTER a SET NOT NULL;
to
ALTER TABLE inh_nn_parent ALTER a SET NOT NULL INHERIT

but we cannot (syntax error)
ALTER TABLE inh_nn_parent ALTER a SET NOT NULL NO INHERIT;

In this case, why not make it no-op, this column "a" already NOT NULL.

so ALTER [ COLUMN ] column_name { SET | DROP } NOT NULL
will change not-null information, no need to consider other not-null
related information.

/*
- * Return the address of the modified column. If the column was already NOT
- * NULL, InvalidObjectAddress is returned.
+ * ALTER TABLE ALTER COLUMN SET NOT NULL
+ *
+ * Add a not-null constraint to a single table and its children. Returns
+ * the address of the constraint added to the parent relation, if one gets
+ * added, or InvalidObjectAddress otherwise.
+ *
+ * We must recurse to child tables during execution, rather than using
+ * ALTER TABLE's normal prep-time recursion.
*/
static ObjectAddress
-ATExecSetNotNull(AlteredTableInfo *tab, Relation rel,
- const char *colName, LOCKMODE lockmode)
+ATExecSetNotNull(List **wqueue, Relation rel, char *conName, char *colName,
+ bool recurse, bool recursing, List **readyRels,
+ LOCKMODE lockmode)

you introduced two boolean "recurse", "recursing", don't have enough
explanation.
That is too much to comprehend.

" * We must recurse to child tables during execution, rather than using
" * ALTER TABLE's normal prep-time recursion.
What does this sentence mean for these two boolean "recurse", "recursing"?

Finally, I did some cosmetic changes, also improved error message
in MergeConstraintsIntoExisting