Hi all,

Attached is my patch that adds DROP CONSTRAINT support to PostgreSQL.  I
basically want your guys feedback.  I have sprinkled some of my q's thru
the text delimited with the @@ symbol.  It seems to work perfectly.

At the moment it does CHECK constraints only, with inheritance.  However,
due to the problem mentioned before with the mismatching between inherited
constraints it may be wise to disable the inheritance feature for a while.
it is written in an extensible fashion to support future dropping of other
types of constraint, and is well documented.

Please send me your comments, check my use of locking, updating of
indices, use of ERROR and NOTICE, etc. and I will rework the patch based
on feedback until everyone
is happy with it...

Chris
Sun, 20 May 2001 15:50:07 +0800 (WST)
Christopher <chriskl@familyhealth.com.au>
DROP CONSTRAINT patch
Anyone looked at this yet?

Also, if someone could tell me where I should attempt to add a regression
test and what, exactly, I should be regression testing it would be
helpful...

Chris

> -----Original Message-----
> From: pgsql-hackers-owner@postgresql.org
> [mailto:pgsql-hackers-owner@postgresql.org]On Behalf Of Christopher
> Sent: Sunday, 20 May 2001 3:50 PM
> To: pgsql-hackers@postgresql.org
> Subject: [HACKERS] DROP CONSTRAINT patch
>
>
> Hi all,
>
> Attached is my patch that adds DROP CONSTRAINT support to PostgreSQL.  I
> basically want your guys feedback.  I have sprinkled some of my q's thru
> the text delimited with the @@ symbol.  It seems to work perfectly.
>
> At the moment it does CHECK constraints only, with inheritance.  However,
> due to the problem mentioned before with the mismatching between inherited
> constraints it may be wise to disable the inheritance feature for a while.
> it is written in an extensible fashion to support future dropping of other
> types of constraint, and is well documented.
>
> Please send me your comments, check my use of locking, updating of
> indices, use of ERROR and NOTICE, etc. and I will rework the patch based
> on feedback until everyone
> is happy with it...
>
> Chris
>


Tue, 22 May 2001 14:25:40 +0800
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au>
RE: DROP CONSTRAINT patch
> Anyone looked at this yet?
> 
> Also, if someone could tell me where I should attempt to add a regression
> test and what, exactly, I should be regression testing it would be
> helpful...

I was hoping someone else would comment on it.  I will check it out
later and either apply or comment.  You seemed little uncertain of it so
I left it in my mailbox for a while.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Tue, 22 May 2001 07:13:24 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: DROP CONSTRAINT patch
No actually, I'm quite confident of it - it works perfectly on my computer
and does exactly what it's supposed to do.  I have tested it extensively.
It also works in a transaction and gets rolled back properly.   However, I
haven't tested it with multiple backends simultaneously doing stuff.  I'm
pretty sure there won't be anything wrong with it though...

It's the stylistic stuff I don't know about.  For example, I issue a NOTICE
if more than one constraint was removed - I don't know if this is desirable
behaviour from the viewpoint of you guys (I personally think it's very
important!).

But hey...

Chris

> -----Original Message-----
> From: Bruce Momjian [mailto:pgman@candle.pha.pa.us]
> Sent: Tuesday, 22 May 2001 7:13 PM
> To: Christopher Kings-Lynne
> Cc: pgsql-hackers@postgresql.org
> Subject: Re: [HACKERS] DROP CONSTRAINT patch
>
>
> > Anyone looked at this yet?
> >
> > Also, if someone could tell me where I should attempt to add a
> regression
> > test and what, exactly, I should be regression testing it would be
> > helpful...
>
> I was hoping someone else would comment on it.  I will check it out
> later and either apply or comment.  You seemed little uncertain of it so
> I left it in my mailbox for a while.
>
> --
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 853-3000
>   +  If your life is a hard drive,     |  830 Blythe Avenue
>   +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
>


Wed, 23 May 2001 09:52:03 +0800
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au>
RE: DROP CONSTRAINT patch

On Tue, 22 May 2001, Christopher Kings-Lynne wrote:

> Anyone looked at this yet?
> 
> Also, if someone could tell me where I should attempt to add a regression
> test and what, exactly, I should be regression testing it would be
> helpful...

At the risk of making it even longer, probably alter_table.sql.
You probably want to try out various conceivable uses of the drop
constraint, including error conditions.

Some things like:
create table with constraint
try to insert valid row
try to insert invalid row
drop the constraint
try to insert valid row
try to insert row that was invalid

create table with two equal named constraints
insert valid to both
insert valid to one but not two
insert valid to two but not one
insert valid to neither
...

create table with two non-equal named constraints
(do inserts)
drop constraint one 
try to insert valid for both, 
  valid for one but not two
  valid for two but not one
  valid for neither
drop constraint two
(do more inserts)


Tue, 22 May 2001 19:59:36 -0700 (PDT)
Stephan Szabo <sszabo@megazone23.bigpanda.com>
RE: DROP CONSTRAINT patch
For the add/drop constraint clauses would it be an idea to change the syntax
to:

ALTER TABLE [ ONLY ] x ADD CONSTRAINT x;
ALTER TABLE [ ONLY ] x DROP CONSTRAINT x;

So that people can specify whether the constraint should be inherited or
not?

Chris


Wed, 23 May 2001 11:21:35 +0800
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au>
ADD/DROP CONSTRAINT and inheritance
Your patch has been added to the PostgreSQL unapplied patches list at:

	http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it withing the next 48 hours.

> Hi all,
> 
> Attached is my patch that adds DROP CONSTRAINT support to PostgreSQL.  I
> basically want your guys feedback.  I have sprinkled some of my q's thru
> the text delimited with the @@ symbol.  It seems to work perfectly.
> 
> At the moment it does CHECK constraints only, with inheritance.  However,
> due to the problem mentioned before with the mismatching between inherited
> constraints it may be wise to disable the inheritance feature for a while.
> it is written in an extensible fashion to support future dropping of other
> types of constraint, and is well documented.
> 
> Please send me your comments, check my use of locking, updating of
> indices, use of ERROR and NOTICE, etc. and I will rework the patch based
> on feedback until everyone
> is happy with it...
> 
> Chris

Content-Description: 

[ Attachment, skipping... ]

> 
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
> 
> http://www.postgresql.org/search.mpl

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Wed, 23 May 2001 11:19:54 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: DROP CONSTRAINT patch
On Wed, 23 May 2001, Christopher Kings-Lynne wrote:

> For the add/drop constraint clauses would it be an idea to change the syntax
> to:
> 
> ALTER TABLE [ ONLY ] x ADD CONSTRAINT x;
> ALTER TABLE [ ONLY ] x DROP CONSTRAINT x;
> 
> So that people can specify whether the constraint should be inherited or
> not?

I'm not sure.  I don't much use the inheritance stuff, so I'm not sure
what would be better for them.  Hopefully one of the people interested
in inheritance will speak up. :)  Technically it's probably not difficult
to do.

A related question is whether or not you can drop a constraint on a
subtable that's inherited from a parent.



Wed, 23 May 2001 09:21:37 -0700 (PDT)
Stephan Szabo <sszabo@megazone23.bigpanda.com>
Re: ADD/DROP CONSTRAINT and inheritance
Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:
> On Wed, 23 May 2001, Christopher Kings-Lynne wrote:
>> For the add/drop constraint clauses would it be an idea to change the syntax
>> to:
>> 
>> ALTER TABLE [ ONLY ] x ADD CONSTRAINT x;
>> ALTER TABLE [ ONLY ] x DROP CONSTRAINT x;

If the patch is coded in the same style as the existing ALTER code then
this will happen automatically.  Are you looking at current development
tip as the comparison point for your changes?

> A related question is whether or not you can drop a constraint on a
> subtable that's inherited from a parent.

There is the question of whether it's a good idea to allow a constraint
to exist on a parent but not on its subtables.  Seems like a bad idea to
me.  But as long as the default is to propagate these changes, I'm not
really eager to prohibit DBAs from doing the other.  Who's to say what's
a misuse of inheritance and what's not...

			regards, tom lane

Wed, 23 May 2001 13:55:21 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: ADD/DROP CONSTRAINT and inheritance 
> Your patch has been added to the PostgreSQL unapplied patches list at:
>
> 	http://candle.pha.pa.us/cgi-bin/pgpatches

Would you like me to submit a version minus all my questions?  And what
about the inheritance feature - should it be left enabled (as is) at the
moment?

Chris


Thu, 24 May 2001 09:34:20 +0800
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au>
RE: DROP CONSTRAINT patch
> Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:
> > On Wed, 23 May 2001, Christopher Kings-Lynne wrote:
> >> For the add/drop constraint clauses would it be an idea to
> change the syntax
> >> to:
> >>
> >> ALTER TABLE [ ONLY ] x ADD CONSTRAINT x;
> >> ALTER TABLE [ ONLY ] x DROP CONSTRAINT x;
>
> If the patch is coded in the same style as the existing ALTER code then
> this will happen automatically.  Are you looking at current development
> tip as the comparison point for your changes?

I'm not sure what you mean here, Tom - I meant that the ONLY keyword could
be optional.  I know that most of the existing ADD CONSTRAINT code does not
propagate constraints to children.  However, if it was ever changed so that
it did - would it be nice to allow the DBA to specify that it should not be
propagated.

> > A related question is whether or not you can drop a constraint on a
> > subtable that's inherited from a parent.
>
> There is the question of whether it's a good idea to allow a constraint
> to exist on a parent but not on its subtables.

It seems to me that someone needs to sit down and decide on the inheritance
semantics that should be enforced (ideally) and then they can be coded to
the design.

> Seems like a bad idea to
> me.  But as long as the default is to propagate these changes, I'm not
> really eager to prohibit DBAs from doing the other.  Who's to say what's
> a misuse of inheritance and what's not...

At the moment we have:

* ADD CONSTRAINT does not propagate
* If you create a table with a CHECK constraint, then create a table that
inherits from that, the CHECK constraint _does_ propagate.

Seems to me that these behaviours are inconsistent...

Chris


Thu, 24 May 2001 10:40:36 +0800
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au>
RE: ADD/DROP CONSTRAINT and inheritance 
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
> I'm not sure what you mean here, Tom - I meant that the ONLY keyword could
> be optional.

The current gram.y code allows either ALTER TABLE foo ONLY or ALTER
TABLE foo* for all forms of ALTER ... with the default interpretation
being the latter.

> At the moment we have:
> * ADD CONSTRAINT does not propagate

I doubt you will find anyone who's willing to argue that that's not a
bug --- specifically, AlterTableAddConstraint()'s lack of inheritance
recursion like its siblings have.  Feel free to fix it.

			regards, tom lane

Wed, 23 May 2001 22:59:53 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: ADD/DROP CONSTRAINT and inheritance 
> "Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
> > I'm not sure what you mean here, Tom - I meant that the ONLY
> keyword could
> > be optional.
>
> The current gram.y code allows either ALTER TABLE foo ONLY or ALTER
> TABLE foo* for all forms of ALTER ... with the default interpretation
> being the latter.

Oops - ok, I didn't notice that...hmmm...maybe I should check my patch for
that before Bruce commits it...

> > At the moment we have:
> > * ADD CONSTRAINT does not propagate
>
> I doubt you will find anyone who's willing to argue that that's not a
> bug --- specifically, AlterTableAddConstraint()'s lack of inheritance
> recursion like its siblings have.  Feel free to fix it.

It was next on my list.  However, I don't want to step on Stephan's toes...

Chris


Thu, 24 May 2001 11:34:06 +0800
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au>
RE: ADD/DROP CONSTRAINT and inheritance 
[ Charset ISO-8859-1 unsupported, converting... ]
> > "Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
> > > I'm not sure what you mean here, Tom - I meant that the ONLY
> > keyword could
> > > be optional.
> >
> > The current gram.y code allows either ALTER TABLE foo ONLY or ALTER
> > TABLE foo* for all forms of ALTER ... with the default interpretation
> > being the latter.
> 
> Oops - ok, I didn't notice that...hmmm...maybe I should check my patch for
> that before Bruce commits it...

Got it.  Patch on hold.

As you can see, inheritance really needs some attention.  Feel free to
give it.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Thu, 24 May 2001 00:29:23 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: ADD/DROP CONSTRAINT and inheritance
> > Your patch has been added to the PostgreSQL unapplied patches list at:
> >
> > 	http://candle.pha.pa.us/cgi-bin/pgpatches
> 
> Would you like me to submit a version minus all my questions?  And what
> about the inheritance feature - should it be left enabled (as is) at the
> moment?

You can send me a totally new patch if you wish.  That way, you can
throw the whole patch to the patches list for comment and improvement.

If you would prefer for me to apply it in pieces, I can.

Please keep going and resolve these inheritance issues.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Thu, 24 May 2001 00:31:40 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: DROP CONSTRAINT patch
> You can send me a totally new patch if you wish.  That way, you can
> throw the whole patch to the patches list for comment and improvement.

I'll do that.

> Please keep going and resolve these inheritance issues.

It's not that there's inheritance issues in my patch.  I am actually certain
that it will work fine with the ONLY keyword.  However, I will make some
changes and submit it to pgsql-patches.

The problem is that although my patch can quite happily remove all
constraints of the same name from all the child tables, the problem is - can
we be sure that all the constraints with that name are actually inherited
from the parent constraint.  Answer - no.  At least, not until the add
constraint stuff is all fixed.  So maybe I will just hash out inheritance
support for the time being.

Chris


Thu, 24 May 2001 12:40:20 +0800
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au>
RE: DROP CONSTRAINT patch
> > You can send me a totally new patch if you wish.  That way, you can
> > throw the whole patch to the patches list for comment and improvement.
> 
> I'll do that.
> 
> > Please keep going and resolve these inheritance issues.
> 
> It's not that there's inheritance issues in my patch.  I am actually certain
> that it will work fine with the ONLY keyword.  However, I will make some
> changes and submit it to pgsql-patches.
> 
> The problem is that although my patch can quite happily remove all
> constraints of the same name from all the child tables, the problem is - can
> we be sure that all the constraints with that name are actually inherited
> from the parent constraint.  Answer - no.  At least, not until the add
> constraint stuff is all fixed.  So maybe I will just hash out inheritance
> support for the time being.

Oh, I was hoping you could at least list _all_ the inheritance issues
for the TODO list.  Seems you have found some already.  If we have that,
we can get a plan for someone to finally fix them all.

What we really need is a list, and a discussion, and decisions, rather
than just letting it lay around.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Thu, 24 May 2001 00:49:32 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: DROP CONSTRAINT patch

> > Seems like a bad idea to
> > me.  But as long as the default is to propagate these changes, I'm not
> > really eager to prohibit DBAs from doing the other.  Who's to say what's
> > a misuse of inheritance and what's not...
> 
> At the moment we have:
> 
> * ADD CONSTRAINT does not propagate
> * If you create a table with a CHECK constraint, then create a table that
> inherits from that, the CHECK constraint _does_ propagate.
> 
> Seems to me that these behaviours are inconsistent...

Yep, but I've got the minimal patch to fix ADD CONSTRAINT.  I'm just
waiting for the upcoming weekend so I can add the regression tests and
pack it up.


Wed, 23 May 2001 21:57:28 -0700 (PDT)
Stephan Szabo <sszabo@megazone23.bigpanda.com>
RE: ADD/DROP CONSTRAINT and inheritance 

Thanks.  Patch applied.  @@ comments removed because patch was reviewed.

> Hi all,
> 
> Attached is my patch that adds DROP CONSTRAINT support to PostgreSQL.  I
> basically want your guys feedback.  I have sprinkled some of my q's thru
> the text delimited with the @@ symbol.  It seems to work perfectly.
> 
> At the moment it does CHECK constraints only, with inheritance.  However,
> due to the problem mentioned before with the mismatching between inherited
> constraints it may be wise to disable the inheritance feature for a while.
> it is written in an extensible fashion to support future dropping of other
> types of constraint, and is well documented.
> 
> Please send me your comments, check my use of locking, updating of
> indices, use of ERROR and NOTICE, etc. and I will rework the patch based
> on feedback until everyone
> is happy with it...
> 
> Chris

Content-Description: 

[ Attachment, skipping... ]

> 
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
> 
> http://www.postgresql.org/search.mpl

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Wed, 30 May 2001 08:57:51 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: DROP CONSTRAINT patch
> 
> Thanks.  Patch applied.  @@ comments removed because patch was reviewed.
> 

Actually, can someone make sure all the @@ comments have been dealt
with?  I assume people checked them, but maybe not.


> > Hi all,
> > 
> > Attached is my patch that adds DROP CONSTRAINT support to PostgreSQL.  I
> > basically want your guys feedback.  I have sprinkled some of my q's thru
> > the text delimited with the @@ symbol.  It seems to work perfectly.
> > 
> > At the moment it does CHECK constraints only, with inheritance.  However,
> > due to the problem mentioned before with the mismatching between inherited
> > constraints it may be wise to disable the inheritance feature for a while.
> > it is written in an extensible fashion to support future dropping of other
> > types of constraint, and is well documented.
> > 
> > Please send me your comments, check my use of locking, updating of
> > indices, use of ERROR and NOTICE, etc. and I will rework the patch based
> > on feedback until everyone
> > is happy with it...
> > 
> > Chris
> 
> Content-Description: 
> 
> [ Attachment, skipping... ]
> 
> > 
> > ---------------------------(end of broadcast)---------------------------
> > TIP 6: Have you searched our list archives?
> > 
> > http://www.postgresql.org/search.mpl
> 
> -- 
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 853-3000
>   +  If your life is a hard drive,     |  830 Blythe Avenue
>   +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
> 

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

? config.log
? config.cache
? config.status
? dropcons.diff
? GNUmakefile
? src/GNUmakefile
? src/Makefile.global
? src/backend/postgres
? src/backend/catalog/global.bki
? src/backend/catalog/global.description
? src/backend/catalog/template1.bki
? src/backend/catalog/template1.description
? src/backend/port/Makefile
? src/bin/initdb/initdb
? src/bin/initlocation/initlocation
? src/bin/ipcclean/ipcclean
? src/bin/pg_config/pg_config
? src/bin/pg_ctl/pg_ctl
? src/bin/pg_dump/pg_dump
? src/bin/pg_dump/pg_restore
? src/bin/pg_dump/pg_dumpall
? src/bin/pg_id/pg_id
? src/bin/pg_passwd/pg_passwd
? src/bin/psql/psql
? src/bin/scripts/createlang
? src/include/config.h
? src/include/stamp-h
? src/interfaces/ecpg/lib/libecpg.so.3
? src/interfaces/ecpg/preproc/ecpg
? src/interfaces/libpgeasy/libpgeasy.so.2
? src/interfaces/libpq/libpq.so.2
? src/pl/plpgsql/src/libplpgsql.so.1
? src/test/regress/log
? src/test/regress/pg_regress
? src/test/regress/tmp_check
? src/test/regress/results
? src/test/regress/expected/copy.out
? src/test/regress/expected/create_function_1.out
? src/test/regress/expected/create_function_2.out
? src/test/regress/expected/misc.out
? src/test/regress/expected/constraints.out
? src/test/regress/sql/copy.sql
? src/test/regress/sql/create_function_1.sql
? src/test/regress/sql/create_function_2.sql
? src/test/regress/sql/misc.sql
? src/test/regress/sql/constraints.sql
Index: src/backend/catalog/heap.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/catalog/heap.c,v
retrieving revision 1.165
diff -c -r1.165 heap.c
*** src/backend/catalog/heap.c	2001/05/14 20:30:19	1.165
--- src/backend/catalog/heap.c	2001/05/20 07:27:27
***************
*** 48,53 ****
--- 48,54 ----
  #include "miscadmin.h"
  #include "optimizer/clauses.h"
  #include "optimizer/planmain.h"
+ #include "optimizer/prep.h"
  #include "optimizer/var.h"
  #include "nodes/makefuncs.h"
  #include "parser/parse_clause.h"
***************
*** 1975,1980 ****
--- 1976,2138 ----
  
  	heap_endscan(rcscan);
  	heap_close(rcrel, RowExclusiveLock);
+ 
+ }
+ 
+ /*
+  * Removes all CHECK constraints on a relation that match the given name.
+  * It is the responsibility of the calling function to acquire a lock on
+  * the relation.
+  * Returns: The number of CHECK constraints removed.
+  */
+ int
+ RemoveCheckConstraint(Relation rel, const char *constrName, bool inh)
+ {
+    Oid            relid;
+ 	Relation			rcrel;
+ 	Relation			relrel;
+ 	Relation			inhrel;
+ 	Relation			relidescs[Num_pg_class_indices];
+ 	TupleDesc		tupleDesc;
+ 	TupleConstr		*oldconstr;
+ 	int				numoldchecks;
+ 	int				numchecks;
+ 	HeapScanDesc	rcscan;
+ 	ScanKeyData		key[2];
+ 	HeapTuple		rctup;
+ 	HeapTuple		reltup;
+ 	Form_pg_class	relStruct;
+ 	int				rel_deleted = 0;
+    int            all_deleted = 0;
+ 
+    /* Find id of the relation */
+    /* @@ Does this need to be done _after_ the heap_open
+       has acquired a lock? @@ */
+    relid = RelationGetRelid(rel);
+ 
+    /* Process child tables and remove constraints of the
+       same name. */
+    if (inh)
+    {
+       List  *child,
+             *children;
+ 
+       /* This routine is actually in the planner */
+       children = find_all_inheritors(relid);
+ 
+       /*
+        * find_all_inheritors does the recursive search of the
+        * inheritance hierarchy, so all we have to do is process all
+        * of the relids in the list that it returns.
+        */
+       foreach(child, children)
+       {
+          Oid	childrelid = lfirsti(child);
+ 
+          if (childrelid == relid)
+             continue;
+          inhrel = heap_open(childrelid, AccessExclusiveLock);
+          all_deleted += RemoveCheckConstraint(inhrel, constrName, false);
+          heap_close(inhrel, NoLock);
+       }
+    }
+ 
+ 	/* Grab an exclusive lock on the pg_relcheck relation */
+ 	rcrel = heap_openr(RelCheckRelationName, RowExclusiveLock);
+ 
+ 	/*
+ 	 * Create two scan keys.  We need to match on the oid of the table
+ 	 * the CHECK is in and also we need to match the name of the CHECK
+ 	 * constraint.
+ 	 */
+ 	ScanKeyEntryInitialize(&key[0], 0, Anum_pg_relcheck_rcrelid,
+ 						   F_OIDEQ, RelationGetRelid(rel));
+ 
+ 	/* @@ F_NAMEEQ correct? @@ */
+ 	ScanKeyEntryInitialize(&key[1], 0, Anum_pg_relcheck_rcname,
+ 						   F_NAMEEQ, PointerGetDatum(constrName));
+ 
+ 	/* @@ &key needed here? @@ */
+ 	/* Begin scanning the heap */
+ 	rcscan = heap_beginscan(rcrel, 0, SnapshotNow, 2, key);
+ 
+ 	/*
+ 	 * Scan over the result set, removing any matching entries.  Note
+ 	 * that this has the side-effect of removing ALL CHECK constraints
+ 	 * that share the specified constraint name.
+ 	 */
+ 	while (HeapTupleIsValid(rctup = heap_getnext(rcscan, 0))) {
+ 		simple_heap_delete(rcrel, &rctup->t_self);
+ 		++rel_deleted;
+       ++all_deleted;
+ 	}
+ 
+ 	/* Clean up after the scan */
+ 	heap_endscan(rcscan);
+ 
+ 	/* @@ Do I need to heap_close here? @@ */
+ 
+ 	/*
+ 	 * @@ I copied this from elsewhere - is it still appropriate? @@
+ 	 * Update the count of constraints in the relation's pg_class tuple.
+ 	 * We do this even if there was no change, in order to ensure that an
+ 	 * SI update message is sent out for the pg_class tuple, which will
+ 	 * force other backends to rebuild their relcache entries for the rel.
+ 	 * (Of course, for a newly created rel there is no need for an SI
+ 	 * message, but for ALTER TABLE ADD ATTRIBUTE this'd be important.)
+ 	 */
+ 
+  	/*
+ 	 * Get number of existing constraints.
+ 	 * @@ Does this need to be above where we delete them!? It seems
+     * to work like it is! @@
+ 	 */
+ 
+ 	tupleDesc = RelationGetDescr(rel);
+ 	oldconstr = tupleDesc->constr;
+ 	if (oldconstr)
+ 		numoldchecks = oldconstr->num_check;
+ 	else
+ 		numoldchecks = 0;
+ 	/* @@ Do we need to pfree oldconstr? @@ */
+ 
+ 	/* Calculate the new number of checks in the table, fail if negative */
+ 	numchecks = numoldchecks - rel_deleted;
+ 
+ 	if (numchecks < 0)
+ 		elog(ERROR, "check count became negative");
+ 
+ 	/* @@ Should I check if rel_deleted > 0 here for
+       efficiency? (re: SI Updates)@@ */
+ 	/* @@ Do I need to heap_open here? @@ */
+ 	relrel = heap_openr(RelationRelationName, RowExclusiveLock);
+ 	reltup = SearchSysCacheCopy(RELOID,
+ 		ObjectIdGetDatum(RelationGetRelid(rel)), 0, 0, 0);
+ 
+ 	if (!HeapTupleIsValid(reltup))
+ 		elog(ERROR, "cache lookup of relation %u failed",
+ 			 RelationGetRelid(rel));
+ 	relStruct = (Form_pg_class) GETSTRUCT(reltup);
+ 
+ 	relStruct->relchecks = numchecks;
+ 
+ 	simple_heap_update(relrel, &reltup->t_self, reltup);
+ 
+ 	/* Keep catalog indices current */
+ 	CatalogOpenIndices(Num_pg_class_indices, Name_pg_class_indices,
+ 					   relidescs);
+ 	CatalogIndexInsert(relidescs, Num_pg_class_indices, relrel, reltup);
+ 	CatalogCloseIndices(Num_pg_class_indices, relidescs);
+ 
+ 	/* Clean up after the scan */
+ 	heap_freetuple(reltup);
+ 	heap_close(relrel, RowExclusiveLock);
+ 
+ 	/* Close the heap relation */
+ 	heap_close(rcrel, RowExclusiveLock);
+ 
+ 	/* Return the number of tuples deleted */
+ 	return all_deleted;
  }
  
  static void
Index: src/backend/commands/command.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/commands/command.c,v
retrieving revision 1.127
diff -c -r1.127 command.c
*** src/backend/commands/command.c	2001/05/09 21:10:38	1.127
--- src/backend/commands/command.c	2001/05/20 07:27:32
***************
*** 51,59 ****
  #include "catalog/pg_shadow.h"
  #include "utils/relcache.h"
  
- #ifdef	_DROP_COLUMN_HACK__
  #include "parser/parse.h"
- #endif	 /* _DROP_COLUMN_HACK__ */
  #include "access/genam.h"
  
  
--- 51,57 ----
***************
*** 1322,1327 ****
--- 1320,1330 ----
  
  							break;
  						}
+ 					case CONSTR_PRIMARY:
+ 						{
+ 
+ 							break;
+ 						}
  					default:
  						elog(ERROR, "ALTER TABLE / ADD CONSTRAINT is not implemented for that constraint type.");
  				}
***************
*** 1583,1595 ****
  
  /*
   * ALTER TABLE DROP CONSTRAINT
   */
  void
  AlterTableDropConstraint(const char *relationName,
  						 bool inh, const char *constrName,
  						 int behavior)
  {
! 	elog(ERROR, "ALTER TABLE / DROP CONSTRAINT is not implemented");
  }
  
  
--- 1586,1657 ----
  
  /*
   * ALTER TABLE DROP CONSTRAINT
+  * Note: It is legal to remove a constraint with name "" as it is possible
+  * to add a constraint with name "".
+  * Christopher Kings-Lynne
   */
  void
  AlterTableDropConstraint(const char *relationName,
  						 bool inh, const char *constrName,
  						 int behavior)
  {
! 	Relation		rel;
! 	int			deleted;
! 
! #ifndef NO_SECURITY
! 	if (!pg_ownercheck(GetUserId(), relationName, RELNAME))
! 		elog(ERROR, "ALTER TABLE: permission denied");
! #endif
! 
! 	/* We don't support CASCADE yet  - in fact, RESTRICT
! 	 * doesn't work to the spec either! */
! 	if (behavior == CASCADE)
! 		elog(ERROR, "ALTER TABLE / DROP CONSTRAINT does not support the CASCADE keyword");
! 
! 	/*
! 	 * Acquire an exclusive lock on the target relation for
! 	 * the duration of the operation.
! 	 * @@ AccessExclusiveLock or RowExclusiveLock??? @@
! 	 */
! 
! 	rel = heap_openr(relationName, AccessExclusiveLock);
! 
! 	/* Disallow DROP CONSTRAINT on views, indexes, sequences, etc */
! 	if (rel->rd_rel->relkind != RELKIND_RELATION)
! 		elog(ERROR, "ALTER TABLE / DROP CONSTRAINT: %s is not a table",
! 			 relationName);
! 
! 	/*
! 	 * Since all we have is the name of the constraint, we have to look through
! 	 * all catalogs that could possibly contain a constraint for this relation.
! 	 * We also keep a count of the number of constraints removed.
! 	 */
! 
! 	deleted = 0;
! 
! 	/*
! 	 * First, we remove all CHECK constraints with the given name
! 	 */
! 
! 	deleted += RemoveCheckConstraint(rel, constrName, inh);
! 
! 	/*
! 	 * Now we remove NULL, UNIQUE, PRIMARY KEY and FOREIGN KEY constraints.
! 	 *
! 	 * Unimplemented.
! 	 */
! 
! 	/* Close the target relation */
! 	heap_close(rel, NoLock);
! 
! 	/* If zero constraints deleted, complain */
! 	if (deleted == 0)
! 		elog(ERROR, "ALTER TABLE / DROP CONSTRAINT: %s does not exist",
! 			 constrName);
! 	/* Otherwise if more than one constraint deleted, notify */
! 	else if (deleted > 1)
! 		elog(NOTICE, "Multiple constraints dropped");
! 
  }
  
  
Index: src/include/catalog/heap.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/catalog/heap.h,v
retrieving revision 1.35
diff -c -r1.35 heap.h
*** src/include/catalog/heap.h	2001/05/07 00:43:24	1.35
--- src/include/catalog/heap.h	2001/05/20 07:27:38
***************
*** 45,50 ****
--- 45,53 ----
  						  List *rawColDefaults,
  						  List *rawConstraints);
  
+ extern int RemoveCheckConstraint(Relation rel, const char *constrName, bool inh);
+ 
+ 
  extern Form_pg_attribute SystemAttributeDefinition(AttrNumber attno);
  
  #endif	 /* HEAP_H */
Wed, 30 May 2001 09:33:03 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: DROP CONSTRAINT patch