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/