cataloguing NOT NULL constraints

From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: cataloguing NOT NULL constraints
Date: 2011-07-07 21:34:01
Message-ID: 20110707213401.GA27098@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

The attached patch introduces pg_constraint rows for NOT NULL
column constraints.

This patch is a heavily reworked version of Bernd Helmle's patch here:
http://archives.postgresql.org/message-id/E57A252DFD60C1FCA91731BD@amenophis

I fixed a bunch of issues with that patch. I think the most
visible one is that NOT NULL constraints are completely separate beasts
from PRIMARY KEYS. This has a number of consequences, including
different inheritance properties (NOT NULL constraints inherit, while
primary keys do not; and ALTER TABLE commands propagate to children
correctly and consistently for both of them); if you drop one of them
but the other remains, the attnotnull marking is not removed from
pg_attribute; if you drop both or if there's only one of them and you
drop it, attnotnull is reset.

Note: if we want to assume the position that PRIMARY KEYS ought to
inherit (that is to say, the NOT NULL bit of them should), we'd need to
discuss that. Not inheriting is a bit of a change in behavior, but
inheriting seems counterintuitive to some.

One bit that was present in Bernd's patch and I left as is is support
for domain NOT NULL constraints. I didn't check this part very much
(which is to say "at all"), so please bear with me if it doesn't work,
bugs are probably mine because of the way I ripped out the underlying
implementation.

I had to invent a new AlterTable command type, because the code to add a
new primary key was injecting a SET NOT NULL command; this didn't work
very well because it automatically created a NOT NULL pg_constraint row,
which obviously isn't what we wanted. So that's AT_SetAttNotNull for
you, which only updates attnotnull without creating a pg_constraint
entry.

(Another thing I changed is that the code no longer uses
CookedConstraint for not null constraints. I invented a new struct to
carry them over. The reason was mostly because MergeAttributes needed
to record attname in some cases and attnum in others, and the Cooked
struct wasn't letting me; instead of changing it, which wasn't a very
good fit for other reasons anyway, it seemed better to invent a new
one more suited to the task.)

Also, there was some attempt to share code to handle DROP NOT NULL with
DROP CONSTRAINT (as discussed in the original thread, see message with
Id D1815DA0B6288201EC1CFAC3(at)amenophis). I ripped that out because it
was rather messy, and since NOT NULL seems to have some particular
requirements anyway, it seems better to have them separate. It's
cleaner and easier to understand anyway. If someone wants to have a go
at merging things, be my guest ...

Note that I'm not adding support for ALTER TABLE / DROP CONSTRAINT to
drop a not null constraint. We can do that of course, I don't think
it's all that hard -- but this patch is large enough already and we can
add that separately. (There's no way to display the NOT NULL constraint
names anyway, though they are preserved on inheritance, per request in
the original threads).

One thing I intend to change before committing is moving all the code I
added to pg_constraint.c back into tablecmds.c (and removing the
accompanying constraint.h header). I thought at one point that this was
better modularity (code that touches pg_constraint should be in the
eponymous file), but seeing how constraints are thoroughly messed up
with in tablecmds.c this seems an exercise in futility. Another change
is eyeballing the new tests a couple more times to ensure

I haven't looked at pg_dump at all, either.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Attachment Content-Type Size
catalog-notnull.patch text/x-diff 133.0 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2011-07-07 21:35:24 Re: SSI atomic commit
Previous Message Heikki Linnakangas 2011-07-07 21:33:28 Re: SSI atomic commit