Re: cataloguing NOT NULL constraints

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Justin Pryzby <pryzby(at)telsasoft(dot)com>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: cataloguing NOT NULL constraints
Date: 2023-04-07 02:14:13
Message-ID: 20230407021413.dolhoh47nrvlthbq@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2023-Apr-06, Justin Pryzby wrote:

> On Thu, Apr 06, 2023 at 01:33:56AM +0200, Alvaro Herrera wrote:
> > - The forms <literal>ADD</literal> (without <literal>USING INDEX</literal>),
> > + The forms <literal>ADD</literal> (without <literal>USING INDEX</literal>, and
> > + except for the <literal>NOT NULL <replaceable>column_name</replaceable></literal>
> > + form to add a table constraint),
> The "except" part seems pretty incoherent to me :(

Yeah, I feared that would be the case. I can't think of a wording
that doesn't take two lines, so suggestions welcome.

I handled your other comments, except these:

> > + conrel = table_open(ConstraintRelationId, RowExclusiveLock);
> Should this be opened after the following error check ?

Added new code in the middle when I found a small problem, so now the
table_open is necessary there. (To wit: if we DROP NOT NULL a
constraint that is both locally defined in the table and inherited, we
should remove the "conislocal" flag and it's done. Previously, we were
throwing an error that the constraint is inherited, but that's wrong.)

> > + arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */
> Does "arr" need to be freed ?

I see this pattern in one or two other places and we don't worry about
such small allocations too much. (I copied this code almost verbatim
from somewhere IIRC).

Anyway, I found a couple of additional minor problems when playing with
some additional corner case scenarios; I cleaned up the test cases, per
Peter. Then I realized that pg_dump support was missing completely, so
I filled that in. Sadly, the binary-upgrade mode is a bit of a mess and
thus the pg_upgrade test is failing.

Álvaro Herrera Breisgau, Deutschland —
"La experiencia nos dice que el hombre peló millones de veces las patatas,
pero era forzoso admitir la posibilidad de que en un caso entre millones,
las patatas pelarían al hombre" (Ijon Tichy)

Attachment Content-Type Size
v7-0001-Catalog-NOT-NULL-constraints.patch text/x-diff 201.4 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2023-04-07 02:18:30 Re: Minimal logical decoding on standbys
Previous Message Tom Lane 2023-04-07 02:09:50 Re: zstd compression for pg_dump