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: Robert Haas <robertmhaas@gmail.com>, Peter Eisentraut <peter@eisentraut.org>, pgsql-hackers@lists.postgresql.org
Date: 2023-08-15T09:57:34Z
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 Fri, 11 Aug 2023 at 14:54, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> Right, in the end I got around to that point of view.  I abandoned the
> idea of adding these dependency links, and I'm back at relying on the
> coninhcount/conislocal markers.  But there were a couple of bugs in the
> accounting for that, so I've fixed some of those, but it's not yet
> complete:
>
> - ALTER TABLE parent ADD PRIMARY KEY
>   needs to create NOT NULL constraints in children.  I added this, but
>   I'm not yet sure it works correctly (for example, if a child already
>   has a NOT NULL constraint, we need to bump its inhcount, but we
>   don't.)
> - ALTER TABLE parent ADD PRIMARY KEY USING index
>   Not sure if this is just as above or needs separate handling
> - ALTER TABLE DROP PRIMARY KEY
>   needs to decrement inhcount or drop the constraint if there are no
>   other sources for that constraint to exist.  I've adjusted the drop
>   constraint code to do this.
> - ALTER TABLE INHERIT
>   needs to create a constraint on the new child, if parent has PK. Not
>   implemented
> - ALTER TABLE NO INHERIT
>   needs to delink any constraints (decrement inhcount, possibly drop
>   the constraint).
>

I think perhaps for ALTER TABLE INHERIT, it should check that the
child has a NOT NULL constraint, and error out if not. That's the
current behaviour, and also matches other constraints types (e.g.,
CHECK constraints).

More generally though, I'm worried that this is starting to get very
complicated. I wonder if there might be a different, simpler approach.
One vague idea is to have a new attribute on the column that counts
the number of constraints (local and inherited PK and NOT NULL
constraints) that make the column not null.

Something else I noticed when reading the SQL standard is that a
user-defined CHECK (col IS NOT NULL) constraint should be recognised
by the system as also making the column not null (setting its
"nullability characteristic" to "known not nullable"). I think that's
more than just an artefact of how they say NOT NULL constraints should
be implemented, because the effect of such a CHECK constraint should
be exposed in the "columns" view of the information schema -- the
value of "is_nullable" should be "NO" if the column is "known not
nullable".

In this sense, the standard does allow multiple not null constraints
on a column, independently of whether the column is "defined as NOT
NULL". My understanding of the standard is that ALTER COLUMN ...
SET/DROP NOT NULL change whether or not the column is "defined as NOT
NULL", and manage a single system-generated constraint, but there may
be any number of other user-defined constraints that also make the
column "known not nullable", and they need to be tracked in some way.

I'm also wondering whether creating a pg_constraint entry for *every*
not-nullable column is actually going too far. If we were to
distinguish between "defined as NOT NULL" and being not null as a
result of one or more constraints, in the way that the standard seems
to suggest, perhaps the former (likely to be much more common) could
simply be a new attribute stored on the column. I think we actually
only need to create pg_constraint entries if a constraint name or any
additional constraint properties such as NOT VALID are specified. That
would lead to far fewer new constraints, less catalog bloat, and less
noise in the \d output.

Regards,
Dean