Re: cataloguing NOT NULL constraints

On Thu, Jul 7, 2011 at 5:34 PM, Alvaro Herrera
<alvherre(at)commandprompt(dot)com> wrote:
> 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.

It would probably be good to add this to the next CommitFest. Not
sure about anyone else, but I'm too busy looking at patches that were
submitted in April, May, and June to look at any new ones right now.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company