|From:||Alvaro Herrera <alvherre(at)commandprompt(dot)com>|
|To:||Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>|
|Cc:||Pg Hackers <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: cataloguing NOT NULL constraints|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
Here's an updated version of this patch. Changes from the previous
* Some more cleanup of the original patch. In particular, domain
constraints have been given a look over. They seem to work fine now.
* information_schema has been updated to display the stored names
instead of inventing fake names. (This probably differs from the
previous behavior in the handling of primary keys).
* pg_dump is supposed to work now.
* The code I said I was going to move from pg_constraint.c to
tablecmds.c has been moved.
Some things I haven't addressed:
Excerpts from Dean Rasheed's message of mié jul 13 04:18:00 -0400 2011:
> I think that there probably ought to be a way to display the NOT NULL
> constraint names (perhaps through \d+). For example, if you're
> planning to support NOT VALID on top of this in the future, then there
> needs to be a way to get the constraint's name to validate it.
Absolutely true. Another thing that needs to be done here is to let the
ALTER TABLE and ALTER DOMAIN commands use the constraint names; right
now, they simply let you add the constraint but not specify the name.
That should probably be revisited.
> You said you haven't looked at pg_dump yet. I'm guessing that the only
> change needed is to dump the NOT NULL constraint name along with the
> table definition so that it is restored correctly.
Yeah, something like that. Domain NOT NULL constraints required a bit
more than that, because the original code assumed that only check
constraints were possible. Actually figuring out how to make that
simple thing work, however, took a lot more effort than it sounds
because pg_dump is so intrincate.
> I had one thought regarding the code: perhaps the NOT NULL constraints
> could be represented using a new struct hanging off the TupleConstr
> struct of a TupleDesc, in the same way as CHECK constraints. Then they
> could be read in by the relcache code at the same time as CHECK
> constraints, rather than by GetRelationNotNullConstraints(), and they
> would be more accessible throughout the backend code. For example,
> supporting NOT VALID on top of this would require a similar change to
> the constraint exclusion code as for CHECK constraints, so the planner
> would need to access the constraint details.
Interesting idea. I haven't touched this.
I'm pretty happy with the current status of this patch and barring
further reviewer comments, I would commit this as is and work on the
improvements proposed by Dean as separate patches.
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
|Next Message||Robert Haas||2011-07-22 00:24:51||Re: sinval synchronization considered harmful|
|Previous Message||Robert Haas||2011-07-21 23:34:58||Re: sinval synchronization considered harmful|