Horton

Re: cataloguing NOT NULL constraints

Dean Rasheed <dean.a.rasheed@gmail.com>

View thread
From: Dean Rasheed <dean.a.rasheed@gmail.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Peter Eisentraut <peter@eisentraut.org>, pgsql-hackers@lists.postgresql.org
Date: 2023-07-24T10:06:06Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Revert structural changes to not-null constraints

  2. Fix inconsistencies in error messages

  3. Disallow direct change of NO INHERIT of not-null constraints

  4. Disallow NO INHERIT not-null constraints on partitioned tables

  5. Better handle indirect constraint drops

  6. Don't try to assign smart names to constraints

  7. Fix restore of not-null constraints with inheritance

  8. ATTACH PARTITION: Don't match a PK with a UNIQUE constraint

  9. Fix propagating attnotnull in multiple inheritance

  10. Check stack depth in new recursive functions

  11. Move privilege check to the right place

  12. Update information_schema definition for not-null constraints

  13. Fix not-null constraint test

  14. Disallow changing NO INHERIT status of a not-null constraint

  15. Catalog not-null constraints

  16. parallel_schedule: add comment on event_trigger test dependency

  17. Revert "Catalog NOT NULL constraints" and fallout

  18. Adjust contrib/sepgsql regression test expected outputs.

  19. Fix table name clash in recently introduced test

  20. Catalog NOT NULL constraints

  21. Change the rules for inherited CHECK constraints to be essentially the same

On Thu, 20 Jul 2023 at 16:31, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2023-Jul-13, Dean Rasheed wrote:
>
> > Something else I noticed is that the result from "ALTER TABLE ...
> > ALTER COLUMN ... DROP NOT NULL" is no longer easily predictable -- if
> > there are multiple NOT NULL constraints on the column, it just drops
> > one (chosen at random?) and leaves the others. I think that it should
> > either drop all the constraints, or throw an error. Either way, I
> > would expect that if DROP NOT NULL succeeds, the result is that the
> > column is nullable.
>
> Hmm, there shouldn't be multiple NOT NULL constraints for the same
> column; if there's one, a further SET NOT NULL should do nothing.  At
> some point the code was creating two constraints, but I realized that
> trying to support multiple constraints caused other problems, and it
> seems to serve no purpose, so I removed it.  Maybe there are ways to end
> up with multiple constraints, but at this stage I would say that those
> are bugs to be fixed, rather than something we want to keep.
>

Hmm, I'm not so sure. I think perhaps multiple NOT NULL constraints on
the same column should just be allowed, otherwise things might get
confusing. For example:

create table p1 (a int not null check (a > 0));
create table p2 (a int not null check (a > 0));
create table foo () inherits (p1, p2);

causes foo to have 2 CHECK constraints, but only 1 NOT NULL constraint:

\d foo
                Table "public.foo"
 Column |  Type   | Collation | Nullable | Default
--------+---------+-----------+----------+---------
 a      | integer |           | not null |
Check constraints:
    "p1_a_check" CHECK (a > 0)
    "p2_a_check" CHECK (a > 0)
Inherits: p1,
          p2

select conname from pg_constraint where conrelid = 'foo'::regclass;
    conname
---------------
 p1_a_not_null
 p2_a_check
 p1_a_check
(3 rows)

which I find a little counter-intuitive / inconsistent. If I then drop
the p1 constraints:

alter table p1 drop constraint p1_a_check;
alter table p1 drop constraint p1_a_not_null;

I end up with column "a" still being not null, and the "p1_a_not_null"
constraint still being there on foo, which seems even more
counter-intuitive, because I just dropped that constraint, and it
really should now be the "p2_a_not_null" constraint that makes "a" not
null:

\d foo
                Table "public.foo"
 Column |  Type   | Collation | Nullable | Default
--------+---------+-----------+----------+---------
 a      | integer |           | not null |
Check constraints:
    "p2_a_check" CHECK (a > 0)
Inherits: p1,
          p2

select conname from pg_constraint where conrelid = 'foo'::regclass;
    conname
---------------
 p1_a_not_null
 p2_a_check
(2 rows)

I haven't thought through various other cases in any detail, but I
can't help feeling that it would be simpler and more logical /
consistent to just allow multiple NOT NULL constraints on a column,
rather than trying to enforce a rule that only one is allowed. That
way, I think it would be easier for the user to keep track of why a
column is not null.

So I'd say that ALTER TABLE ... ADD NOT NULL should always add a
constraint, even if there already is one. For example ALTER TABLE ...
ADD UNIQUE does nothing to prevent multiple unique constraints on the
same column(s). It seems pretty dumb, but maybe there is a reason to
allow it, and it doesn't feel like we should be second-guessing what
the user wants.

Regards,
Dean