Thread: 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
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 >
> 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, Pennsylvania19026
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 >
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)
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
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, Pennsylvania19026
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.
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
> 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
> 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
"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
> "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
> > 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, Pennsylvania19026
[ 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, Pennsylvania19026
> 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
> > 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, Pennsylvania19026
> > 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.
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, Pennsylvania19026
> > 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 */