Re: not null constraints, again
jian he <jian.universality@gmail.com>
From: jian he <jian.universality@gmail.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Tender Wang <tndrwang@gmail.com>,
Pg Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-09-19T09:26:59Z
Lists: pgsql-hackers
On Thu, Sep 19, 2024 at 4:26 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
>
> > drop table if exists idxpart, idxpart0, idxpart1 cascade;
> > create table idxpart (a int) partition by range (a);
> > create table idxpart0 (a int primary key);
> > alter table idxpart attach partition idxpart0 for values from (0) to (1000);
> > alter table idxpart alter column a set not null;
> > alter table idxpart0 alter column a drop not null;
> > alter table idxpart0 drop constraint idxpart0_a_not_null;
> >
> > "alter table idxpart0 alter column a drop not null;"
> > is logically equivalent to
> > "alter table idxpart0 drop constraint idxpart0_a_not_null;"
> >
> > the first one (alter column) ERROR out,
> > the second success.
> > the second "drop constraint" should also ERROR out?
> > since it violates the sentence in ddl-partitioning.html
> > "You cannot drop a NOT NULL constraint on a partition's column if the
> > same constraint is present in the parent table."
>
> Yeah, I modified this code already a few days ago, and now it does error
> out like this
>
> ERROR: cannot drop inherited constraint "idxpart0_a_not_null" of relation "idxpart0"
>
> Anyway, as I mentioned back then, the DROP CONSTRAINT didn't _actually_
> remove the constraint; it only marked the constraint as no longer
> locally defined (conislocal=false), which had no practical effect other
> than changing the representation during pg_dump. Even detaching the
> partition after having "dropped" the constraint would make the not-null
> constraint appear again as coninhcount=0,conislocal=true rather than
> drop it.
>
funny.
as the previously sql example, if you execute
"alter table idxpart0 drop constraint idxpart0_a_not_null;"
again
then
ERROR: cannot drop inherited constraint "idxpart0_a_not_null" of
relation "idxpart0"
I am not sure if that's logically OK or if the user can deduce the
logic from the manual.
like, the first time you use "alter table drop constraint"
to drop a constraint, the constraint is not totally dropped,
the second time you execute it again the constraint cannot be dropped directly.
i think the issue is the changes we did in dropconstraint_internal
in dropconstraint_internal, we have:
-----------
if (con->contype == CONSTRAINT_NOTNULL &&
con->conislocal && con->coninhcount > 0)
{
HeapTuple copytup;
copytup = heap_copytuple(constraintTup);
con = (Form_pg_constraint) GETSTRUCT(copytup);
con->conislocal = false;
CatalogTupleUpdate(conrel, ©tup->t_self, copytup);
ObjectAddressSet(conobj, ConstraintRelationId, con->oid);
CommandCounterIncrement();
table_close(conrel, RowExclusiveLock);
return conobj;
}
/* Don't allow drop of inherited constraints */
if (con->coninhcount > 0 && !recursing)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
errmsg("cannot drop inherited constraint \"%s\" of
relation \"%s\"",
constrName, RelationGetRelationName(rel))));
-----------
comments in dropconstraint_internal
"* Reset pg_constraint.attnotnull, if this is a not-null constraint."
should be
"pg_attribute.attnotnull"
also, we don't have tests for not-null constraint similar to check
constraint tests on
src/test/regress/sql/alter_table.sql (line 2067 to line 2073)
Commits
-
Suppress "may be used uninitialized" warnings from older compilers.
- fc5e966f73f0 18.0 landed
-
Elide not-null constraint checks on child tables during PK creation
- 11ff192b5bb7 18.0 landed
-
Remove unnecessary code to handle CONSTR_NOTNULL
- 5b291d1c9c09 18.0 landed
-
Silence compilers about extractNotNullColumn()
- ff239c3bf4e8 18.0 landed
-
Add pg_constraint rows for not-null constraints
- 14e87ffa5c54 18.0 landed