[badalex@gmail.com: Re: [BUGS] Problem identifying constraints which should not be inherited] - Mailing list pgsql-patches

Forwarding this message so that it gets archived.

--
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Thu, Mar 27, 2008 at 5:14 AM, NikhilS <nikkhils@gmail.com> wrote:
> Hi,
>
> On Thu, Mar 20, 2008 at 7:36 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >
> >
> > More to the point, it takes a capability away from the user without
> > actually solving the problem we need to solve, namely to guarantee
> > consistency between parent and child constraints.  You can be sure
> > that there is someone out there who will complain that we've broken
> > his application when we disallow this, and we need to be able to
> > point to some positive benefit we got from it.
> >
>
> Agreed. So I think we need to implement the whole enchilada or nothing at
> all. We need to do the following for this:
>
> * Add logic to disallow ADD CONSTRAINT ONLY to parent of an inheritance
> hierarchy
>
> * Add logic to mark inherited constraints in the children:
> This can be achieved by introducing a new bool "coninherited" attribute in
> pg_constraint. This will be set to true on only those check constraints that
> are added to children via the inheritance mechanism
>
> * Add logic to disallow dropping inherited constraints directly on children
> Obviously they will get dropped if a DROP CONSTRAINT is fired on the parent.
> with recurse set to true (this is the default behaviour)
>
>  * Modify the pg_dump logic to use the new pg_constraint based attribute
> logic for version 80300 (or should it be PG_VERSION_NUM 80400?) onwards.
> Infact the current logic to determine if a check constraint is inherited is
> to compare its name with the parent constraint name and the comment already
> mentions that we should make changes in pg_constraint to avoid this
> rudimentary way of determing the inheritance :).
>
> Am important decision here is about adding a new attribute to pg_constraint
> as it is the only sane way of determining inherited constraints, but that
> will require an initdb. Comments?
>

Attached is a WIP patch I have been playing with in my spare time.  It
should take care of the first 2.  It does nothing for pg_dump or set
(not) null/set default.

Note it has some gross points (see comment in
src/backend/catalog/heap.c AddRelationRawConstraints) and the
regression tests I added are not quite up to par (and probably a bit
redundant).  But in the interest of saving work I thought i would post
it.

famous last words "It seems to work"...

diff stat for those that care:

 src/backend/catalog/heap.c                |   55 ++++-
 src/backend/catalog/index.c               |    4 +-
 src/backend/catalog/pg_constraint.c       |   51 ++++-
 src/backend/commands/tablecmds.c          |  366 ++++++++++++++++++----------
 src/backend/commands/typecmds.c           |    4 +-
 src/backend/nodes/copyfuncs.c             |    2 +
 src/backend/nodes/equalfuncs.c            |    2 +
 src/backend/nodes/outfuncs.c              |    3 +
 src/backend/parser/gram.y                 |    4 +
 src/backend/parser/parse_utilcmd.c        |    5 +
 src/backend/utils/cache/catcache.c        |    2 +-
 src/backend/utils/cache/syscache.c        |   12 +
 src/include/access/tupdesc.h              |    6 +-
 src/include/catalog/catversion.h          |    2 +-
 src/include/catalog/heap.h                |    1 +
 src/include/catalog/indexing.h            |    2 +
 src/include/catalog/pg_constraint.h       |   59 +++--
 src/include/nodes/parsenodes.h            |    4 +-
 src/include/utils/syscache.h              |   99 ++++----
 src/test/regress/expected/alter_table.out |    2 +-
 src/test/regress/expected/inherit.out     |   90 +++++++
 src/test/regress/sql/inherit.sql          |   45 ++++
 22 files changed, 605 insertions(+), 215 deletions(-)