Feedback-ID: ia2694551:Fastmail
Date: Fri, 4 Aug 2023 20:10:42 +0200
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>,
	Peter Eisentraut <peter@eisentraut.org>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: cataloguing NOT NULL constraints
Message-ID: <20230804181042.3hydj5sy2aeapdia@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20230728104744.wvolubqxph3ag5hh@alvherre.pgsql>
Archived-At: 
 <https://www.postgresql.org/message-id/20230804181042.3hydj5sy2aeapdia%40alvherre.pgsql>
Precedence: bulk

On 2023-Jul-28, Alvaro Herrera wrote:

> To avoid that, one option would be to make this NN constraint
> undroppable ...  but I don't see how.  One option might be to add a
> pg_depend row that links the NOT NULL constraint to its PK constraint.
> But this will be a strange case that occurs nowhere else, since other
> NOT NULL constraint don't have such pg_depend rows.  Also, I won't know
> how pg_dump likes this until I implement it.

I've been completing the implementation for this.  It seems to work
reasonably okay; pg_dump requires somewhat strange contortions, but they
are similar to what we do in flagInhTables already, so I don't feel too
bad about that.

What *is* odd and bothersome is that it also causes a problem dropping
the child table.  For example,

CREATE TABLE parent (a int primary key);
CREATE TABLE child () INHERITS (parent);
\d+ child

                                                 Tabla «public.child»
 Columna │  Tipo   │ Ordenamiento │ Nulable  │ Por omisión │ Almacenamiento │ Compresión │ Estadísticas │ Descripción 
─────────┼─────────┼──────────────┼──────────┼─────────────┼────────────────┼────────────┼──────────────┼─────────────
 a       │ integer │              │ not null │             │ plain          │            │              │ 
Not null constraints:
    "child_a_not_null" NOT NULL "a"
Hereda: parent
Método de acceso: heap

This is the behavior that I think we wanted to prevent drop of the child
constraint, and it seems okay to me:

=# alter table child drop constraint child_a_not_null;
ERROR:  cannot drop constraint child_a_not_null on table child because constraint parent_pkey on table parent requires it
SUGERENCIA:  You can drop constraint parent_pkey on table parent instead.

But the problem is this:

=# drop table child;
ERROR:  cannot drop table child because other objects depend on it
DETALLE:  constraint parent_pkey on table parent depends on table child
SUGERENCIA:  Use DROP ... CASCADE to drop the dependent objects too.


To be clear, what my patch is doing is add one new dependency:

                    dep                     │                  ref                   │ deptype 
────────────────────────────────────────────┼────────────────────────────────────────┼─────────
 type foo                                   │ table foo                              │ i
 table foo                                  │ schema public                          │ n
 constraint foo_pkey on table foo           │ column a of table foo                  │ a
 type bar                                   │ table bar                              │ i
 table bar                                  │ schema public                          │ n
 table bar                                  │ table foo                              │ n
 constraint bar_a_not_null on table bar     │ column a of table bar                  │ a
 constraint child_a_not_null on table child │ column a of table child                │ a
 constraint child_a_not_null on table child │ constraint parent_pkey on table parent │ i

the last row here is what is new.  I'm not sure what's the right fix.
Maybe I need to invert the direction of that dependency.


Even with that fixed, I'd still need to write more code so that ALTER
TABLE INHERIT adds the link (I already patched the DROP INHERIT part).
Not sure what else might I be missing.

Separately, I also noticed that some code that's currently
dropconstraint_internal needs to be moved to DropConstraintById, because
if the PK is dropped for some other reason than ALTER TABLE DROP
CONSTRAINT, some ancillary actions are not taken.

Sigh.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
 Are you not unsure you want to delete Firefox?
       [Not unsure]     [Not not unsure]    [Cancel]
                   http://smylers.hates-software.com/2008/01/03/566e45b2.html