Horton

Re: cataloguing NOT NULL constraints

Alvaro Herrera <alvherre@alvh.no-ip.org>

View thread
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Dean Rasheed <dean.a.rasheed@gmail.com>, Peter Eisentraut <peter@eisentraut.org>
Cc: Robert Haas <robertmhaas@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2023-08-21T18:01:01Z
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

Attachments

Okay, so here's another version of this, where I fixed the creation of
NOT NULLs derived from PKs.  It turned out that what I was doing wasn't
doing recursion correctly, so for example if you have NOT NULLs in
grand-child tables they would not be marked as inherited from the PK
(thus wrongly droppable).  I had to rewrite it to go through ATPrepCmd
and friends; and we had no way to indicate inheritance that way, so I
had to add an "int inhcount" to the Constraint node.  (I think it might
be OK to make it just a "bool inherited" instead).

There is one good thing about this, which is that currently
AddRelationNewConstraints() has a strange "bool is_local" parameter
(added by commit cd902b331d, 2008), which is somewhat strange, and which
we could remove to instead use this new Constraint->inhcount mechanism
to pass down the flag.


Also: it turns out that you can do this
CREATE TABLE parent (a int);
CREATE TABLE child (NOT NULL a) INHERITS (parent);

that is, the column has no local definition on the child, but the
constraint does.  This required some special fixes but also works
correctly now AFAICT.

On 2023-Aug-16, Peter Eisentraut wrote:

> I have two small patches that you can integrate into your patch set:
> 
> The first just changes the punctuation of "Not-null constraints" in the psql
> output to match what the documentation mostly uses.
> 
> The second has some changes to ddl.sgml to reflect that not-null constraints
> are now named and can be operated on like other constraints.  You might want
> to read that again to make sure it matches your latest intentions, but I
> think it catches all the places that are required to change.

I've incorporated both of those, verbatim for now; I'll give the docs
another look tomorrow.

On 2023-Aug-11, Alvaro Herrera wrote:

> - 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 also need to add tests for those scenarios, because I think there
> aren't any for most of them.

I've added tests for the ones I caught missing, including leaving some
tables to exercise the pg_upgrade side of things.

> There's also another a pg_upgrade problem: we now get spurious ALTER
> TABLE SET NOT NULL commands in a dump after pg_upgrade for the columns
> that get the constraint from a primary key.

I fixed this too.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/