Re: not null constraints, again

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: Tender Wang <tndrwang(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: not null constraints, again
Date: 2024-09-26 16:53:01
Message-ID: 202409261653.zrz32zjsaqh4@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2024-Sep-26, jian he wrote:

> +-- a PK in parent must have a not-null in child that it can mark inherited
> +create table inh_parent (a int primary key);
> +create table inh_child (a int primary key);
> +alter table inh_child inherit inh_parent; -- nope
> +alter table inh_child alter a set not null;
> +alter table inh_child inherit inh_parent; -- now it works
> +ERROR: relation "inh_parent" would be inherited from more than once
> in src/test/regress/sql/inherit.sql, the comments at the end of the
> command, seem to conflict with the output?

Outdated, useless -- removed.

> -------------------------------------------------------------------------------
>
> ALTER TABLE ALTER COLUMN SET NOT NULL
> implicitly means
> ALTER TABLE ALTER COLUMN SET NOT NULL NO INHERIT.
>
> So in ATExecSetNotNull
> if (conForm->connoinherit && recurse)
> ereport(ERROR,
> errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> errmsg("cannot change NO INHERIT status of NOT
> NULL constraint \"%s\" on relation \"%s\"",
> NameStr(conForm->conname),
> RelationGetRelationName(rel)));
> should be
> if (conForm->connoinherit)
> ereport(ERROR,
> errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> errmsg("cannot change NO INHERIT status of NOT
> NULL constraint \"%s\" on relation \"%s\"",
> NameStr(conForm->conname),
> RelationGetRelationName(rel)));
>
> then we can avoid the weird case like below:
>
> drop table if exists pp1;
> create table pp1 (f1 int not null no inherit);
> ALTER TABLE pp1 ALTER f1 SET NOT NULL;
> ALTER TABLE ONLY pp1 ALTER f1 SET NOT NULL;

Hmm, I don't understand why you say SET NOT NULL implicitly means SET
NOT NULL NO INHERIT. That's definitely not the intention. As I
explained earlier, the normal state is that a constraint is inheritable,
so if you do SET NOT NULL you want that constraint to be INHERIT.

Anyway, I don't see what you see as weird in the commands you list. To
me it reacts like this:

=# create table pp1 (f1 int not null no inherit);
CREATE TABLE
=# ALTER TABLE pp1 ALTER f1 SET NOT NULL;
ERROR: cannot change NO INHERIT status of NOT NULL constraint "pp1_f1_not_null" on relation "pp1"
=# ALTER TABLE ONLY pp1 ALTER f1 SET NOT NULL;
ALTER TABLE
=# \d+ pp1
Tabla «public.pp1»
Columna │ Tipo │ Ordenamiento │ Nulable │ Por omisión │ Almacenamiento │ Compresión │ Estadísticas │ Descripción
─────────┼─────────┼──────────────┼──────────┼─────────────┼────────────────┼────────────┼──────────────┼─────────────
f1 │ integer │ │ not null │ │ plain │ │ │
Not-null constraints:
"pp1_f1_not_null" NOT NULL "f1" NO INHERIT
Método de acceso: heap

which seems to be exactly what we want.

> -------------------------------------------------------------------------------
>
> + else if (rel->rd_rel->relhassubclass &&
> + find_inheritance_children(RelationGetRelid(rel), NoLock) != NIL)
> + {
> + ereport(ERROR,
> + errcode(ERRCODE_INVALID_TABLE_DEFINITION),
> + errmsg("not-null constraint on column \"%s\" must be removed in
> child tables too",
> + colName),
> + errhint("Do not specify the ONLY keyword."));
> + }
> this part in ATExecDropNotNull is not necessary?
>
> per alter_table.sql
> <<<<<<---------->>>>>>
> -- make sure we can drop a constraint on the parent but it remains on the child
> CREATE TABLE test_drop_constr_parent (c text CHECK (c IS NOT NULL));
> CREATE TABLE test_drop_constr_child () INHERITS (test_drop_constr_parent);
> ALTER TABLE ONLY test_drop_constr_parent DROP CONSTRAINT
> "test_drop_constr_parent_c_check";
> <<<<<<---------->>>>>>
> by the same way, we can drop a not-null constraint ONLY on the parent,
> but it remains on the child.
> if we not remove the above part then
> ALTER TABLE ONLY DROP CONSTRAINT
> will behave differently from
> ALTER TABLE ONLY ALTER COLUMN DROP NOT NULL.
>
> example:
> 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);
>
> alter table only pp1 drop constraint pp1_f1_not_null; --works.
> alter table only pp1 alter column f1 drop not null; --- error, should also work.
> -------------------------------------------------------------------------------

Hmm. I'm not sure I like this behavior, but there is precedent in
CHECK, and since DROP CONSTRAINT also already works that way, I suppose
DROP NOT NULL should do that too. I'll get it changed.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"I must say, I am absolutely impressed with what pgsql's implementation of
VALUES allows me to do. It's kind of ridiculous how much "work" goes away in
my code. Too bad I can't do this at work (Oracle 8/9)." (Tom Allison)
http://archives.postgresql.org/pgsql-general/2007-06/msg00016.php

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2024-09-26 17:16:38 Re: SQL:2023 JSON simplified accessor support
Previous Message Tom Lane 2024-09-26 16:39:10 Re: [PATCH] Support Int64 GUCs