Re: cataloguing NOT NULL constraints

From: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: cataloguing NOT NULL constraints
Date: 2023-07-26 13:09:02
Message-ID: CAEZATCWDCy8hCe_L7mPVfONmRnx7QeLu-vfH9DW-STw++KXtrA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, 25 Jul 2023 at 13:36, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>
> Okay then, I've made these show up in the footer of \d+. This is in
> patch 0003 here. Please let me know what do you think of the regression
> changes.
>

The new \d+ output certainly makes testing and reviewing easier,
though I do understand people's concerns that this may make the output
significantly longer in many real-world cases. I don't think it would
be a good idea to filter the list in any way though, because I think
that will only lead to confusion. I think it should be all-or-nothing,
though I'm not necessarily opposed to using something like \d++ to
enable it, if that turns out to be the least-bad option.

Going back to this example:

drop table if exists p1, p2, foo;
create table p1(a int not null check (a > 0));
create table p2(a int not null check (a > 0));
create table foo () inherits (p1,p2);
\d+ foo

Table "public.foo"
Column | Type | Collation | Nullable | Default | Storage |
Compression | Stats target | Description
--------+---------+-----------+----------+---------+---------+-------------+--------------+-------------
a | integer | | not null | | plain |
| |
Check constraints:
"p1_a_check" CHECK (a > 0)
"p2_a_check" CHECK (a > 0)
Not null constraints:
"p1_a_not_null" NOT NULL "a" (inherited)
Inherits: p1,
p2
Access method: heap

I remain of the opinion that that should create 2 NOT NULL constraints
on foo, for consistency with CHECK constraints, and the misleading
name that results if p1_a_not_null is dropped from p1. That way, the
names of inherited NOT NULL constraints could be kept in sync, as they
are for other constraint types, making it easier to keep track of
where they come from, and it wouldn't be necessary to treat them
differently (e.g., matching by column number, when dropping NOT NULL
constraints).

Doing a little more testing, I found some other issues.

Given the following sequence:

drop table if exists p,c;
create table p(a int primary key);
create table c() inherits (p);
alter table p drop constraint p_pkey;

p.a ends up being nullable, where previously it would have been left
non-nullable. That change makes sense, and is presumably one of the
benefits of tying the nullability of columns to pg_constraint entries.
However, c.a remains non-nullable, with a NOT NULL constraint that
claims to be inherited:

\d+ c
Table "public.c"
Column | Type | Collation | Nullable | Default | Storage |
Compression | Stats target | Description
--------+---------+-----------+----------+---------+---------+-------------+--------------+-------------
a | integer | | not null | | plain |
| |
Not null constraints:
"c_a_not_null" NOT NULL "a" (inherited)
Inherits: p
Access method: heap

That's a problem, because now the NOT NULL constraint on c cannot be
dropped (attempting to drop it on c errors out because it thinks it's
inherited, but it can't be dropped via p, because p.a is already
nullable).

I wonder if NOT NULL constraints created as a result of inherited PKs
should have names based on the PK name (e.g.,
<PK_name>_<col_name>_not_null), to make it more obvious where they
came from. That would be more consistent with the way NOT NULL
constraint names are inherited.

Given the following sequence:

drop table if exists p,c;
create table p(a int);
create table c() inherits (p);
alter table p add primary key (a);

c.a ends up non-nullable, but there is no pg_constraint entry
enforcing the constraint:

\d+ c
Table "public.c"
Column | Type | Collation | Nullable | Default | Storage |
Compression | Stats target | Description
--------+---------+-----------+----------+---------+---------+-------------+--------------+-------------
a | integer | | not null | | plain |
| |
Inherits: p
Access method: heap

Given a database containing these 2 tables:

create table p(a int primary key);
create table c() inherits (p);

doing a pg_dump and restore fails to restore the NOT NULL constraint
on c, because all constraints created by the dump are local to p.

That's it for now. I'll try to do more testing later.

Regards,
Dean

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Matthias van de Meent 2023-07-26 13:16:02 Re: incremental-checkopints
Previous Message Aleksander Alekseev 2023-07-26 13:02:11 Re: Remove unused fields in ReorderBufferTupleBuf