Re: cataloguing NOT NULL constraints

On 2023-Jun-30, Andres Freund wrote:

> On 2023-06-30 13:44:03 +0200, Alvaro Herrera wrote:
> 
> > The main novelty in this version of the patch, is that we now emit
> > "throwaway" NOT NULL constraints when a column is part of the primary
> > key.  Then, after the PK is created, we run a DROP for that constraint.
> > That lets us create the PK without having to scan the table during
> > pg_upgrade.
> 
> Have you considered extending the DDL statement for this purpose? We have
>   ALTER TABLE ... ADD CONSTRAINT ... PRIMARY KEY USING INDEX ...;
> we could just do something similar for the NOT NULL constraint?  Which would
> then delete the separate constraint NOT NULL constraint.

Hmm, I hadn't.  I think if we have to explicitly list the constraint
that we want dropped, then it's pretty much the same than as if we used
a comma-separated list of subcommands, like 

ALTER TABLE ... ADD CONSTRAINT .. PRIMARY KEY (a,b),
   DROP CONSTRAINT pgdump_throwaway_notnull_0,
   DROP CONSTRAINT pgdump_throwaway_notnull_1;

However, I think it would be ideal if we *don't* have to specify the
list of constraints: we would do this on any ALTER TABLE .. ADD
CONSTRAINT PRIMARY KEY, without having any additional clause.

But how to distinguish which NOT NULL markings to drop?  Maybe we would
have to specify a flag at NOT NULL constraint creation time.  So pg_dump
would emit something like

CREATE TABLE foo (a int CONSTRAINT NOT NULL THROWAWAY);
... (much later) ...
ALTER TABLE foo ADD CONSTRAINT .. PRIMARY KEY;

and by the time this second command is run, those throwaway constraints
are removed.  The problems now are 1) how to make this CREATE statement
more SQL-conformant (answer: make pg_dump emit a separate ALTER TABLE
command for the constraint addition; it already knows how to do this, so
it'd be very little code); but also 2) where to store the flag
server-side flag that says this constraint has this property.  I think
it'd have to be a new pg_constraint column, and I don't like to add one
for such a minor issue.

On 2023-Jun-30, Alvaro Herrera wrote:

> Scanning this thread, I think I left one reported issue unfixed related
> to tables created LIKE others.  I'll give it a look later.  Other than
> that I think all bases are covered, but I intend to leave the patch open
> until near the end of the CF, in case someone wants to play with it.

So it was [1] that I meant, where this example was provided:

# create table t1 (c int primary key null unique);
                                             
 
# create table t2 (like t1);
                                             
 
# alter table t2 alter c drop not null;
                                             
 
ERROR:  no NOT NULL constraint found to drop

The problem here is that because we didn't give INCLUDING INDEXES in the
LIKE clause, we end up with a column marked NOT NULL for which we have
no pg_constraint row.  Okay, I thought, we can just make sure *not* to
mark that case as not null; that works fine and looks reasonable.
However, it breaks the following use case, which is already in use in
the regression tests and possibly by users:

 CREATE TABLE pk (a int PRIMARY KEY) PARTITION BY RANGE (a);
 CREATE TABLE pk4 (LIKE pk);
 ALTER TABLE pk ATTACH PARTITION pk4 FOR VALUES FROM (3000) TO (4000);
+ERROR:  column "a" in child table must be marked NOT NULL

The problem here is that we were assuming, by the time the third command
is run, that the column had been marked NOT NULL by the second command.
So my solution above is simply not acceptable.  What we must do, in
order to handle this backward-compatibly, is to ensure that a column
part of a PK automatically gets a NOT NULL constraint for all the PK
columns, for the case where INCLUDING INDEXES is not given.  This is the
same we do for regular INHERITS children and PKs.

I'll go write this code now; should be simple enough.

[1] https://postgr.es/m/CAMbWs48astPDb3K+L89wb8Yju0jM_Czm8svmU=Uzd+WM61Cr6Q@mail.gmail.com

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
<Schwern> It does it in a really, really complicated way
<crab> why does it need to be complicated?
<Schwern> Because it's MakeMaker.