Обсуждение: Patch for bug #2073 (Can't drop sequence when created via SERIAL column)
Patch for bug #2073 (Can't drop sequence when created via SERIAL column)
От
Dhanaraj M - Sun Microsystems
Дата:
Hi all I fixed the above bug. I attach the patch here. Please review and acknowledge me. Bug details ======== BUG #2073: Can't drop sequence when created via SERIAL column * From: "Aaron Dummer" <aaron ( at ) dummer ( dot ) info> * To: pgsql-bugs ( at ) postgresql ( dot ) org * Subject: BUG #2073: Can't drop sequence when created via SERIAL column * Date: Mon, 28 Nov 2005 21:10:33 +0000 (GMT) The following bug has been logged online: Bug reference: 2073 Logged by: Aaron Dummer Email address: aaron ( at ) dummer ( dot ) info PostgreSQL version: 8.0.3 Operating system: Debian Linux Description: Can't drop sequence when created via SERIAL column Details: If I create a table named foo with a column named bar, column type SERIAL, it auto-generates a sequence named foo_bar_seq. Now if I manually create a new sequence called custom_seq, and change the default value of foo.bar to reference the new sequence, I still can't delete the old sequence (foo_bar_seq). In other words, from a user's point of view, the foo table is no longer dependent on the foo_bar_seq, yet the system still sees it as dependent. I brought this topic up on the #postgresql IRC channel and the behavior was confirmed by AndrewSN, scampbell_, and mastermind. -------------------------------------------- # 1 - create the table with a SERIAL column beta=# CREATE TABLE test_table (id serial, value text); NOTICE: CREATE TABLE will create implicit sequence "test_table_id_seq" for serial column "test_table.id" CREATE TABLE # 2 - create the new sequence you want to use beta=# CREATE SEQUENCE new_sequence_seq; CREATE SEQUENCE #3 - alter the table so it uses the new sequence you made beta=# ALTER TABLE test_table ALTER COLUMN id SET DEFAULT nextval('new_sequence_seq'); ALTER TABLE #4 - try to delete the test_table_id_seq, since it's not used anymore beta=# DROP SEQUENCE test_table_id_seq; ERROR: cannot drop sequence test_table_id_seq because table test_table column id requires it HINT: You may drop table test_table column id instead. You see, PostgreSQL used some underlying mechanism for noting that test_table depends on test_table_id_seq. It shouldn't do that. *** ./src/backend/catalog/dependency.c.orig Mon Apr 10 13:47:39 2006 --- ./src/backend/catalog/dependency.c Mon Apr 10 14:39:33 2006 *************** *** 1931,1933 **** --- 1931,2009 ---- ReleaseSysCache(relTup); } + + /* Recursively travel and search for the default sequence. Finally detach it */ + + void performSequenceDefaultDeletion(const ObjectAddress *object, + DropBehavior behavior, int deleteFlag) + { + + ScanKeyData key[3]; + int nkeys; + SysScanDesc scan; + HeapTuple tup; + ObjectAddress otherObject; + Relation depRel; + + depRel = heap_open(DependRelationId, RowExclusiveLock); + + ScanKeyInit(&key[0], + Anum_pg_depend_classid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(object->classId)); + ScanKeyInit(&key[1], + Anum_pg_depend_objid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(object->objectId)); + if (object->objectSubId != 0) + { + ScanKeyInit(&key[2], + Anum_pg_depend_objsubid, + BTEqualStrategyNumber, F_INT4EQ, + Int32GetDatum(object->objectSubId)); + nkeys = 3; + } + else + nkeys = 2; + + scan = systable_beginscan(depRel, DependDependerIndexId, true, + SnapshotNow, nkeys, key); + + while (HeapTupleIsValid(tup = systable_getnext(scan))) + { + + Form_pg_depend foundDep = (Form_pg_depend) GETSTRUCT(tup); + + otherObject.classId = foundDep->refclassid; + otherObject.objectId = foundDep->refobjid; + otherObject.objectSubId = foundDep->refobjsubid; + + /* Detach the default sequence from the relation */ + if(deleteFlag == 1) + { + simple_heap_delete(depRel, &tup->t_self); + break; + } + + switch (foundDep->deptype) + { + case DEPENDENCY_NORMAL: + { + + if(getObjectClass(&otherObject) == OCLASS_CLASS) + { + performSequenceDefaultDeletion(&otherObject, behavior, 1); + systable_endscan(scan); + heap_close(depRel, RowExclusiveLock); + return; + } + } + + } + } + + systable_endscan(scan); + heap_close(depRel, RowExclusiveLock); + + } + *** ./src/backend/catalog/heap.c.orig Mon Apr 10 13:47:51 2006 --- ./src/backend/catalog/heap.c Mon Apr 10 14:39:59 2006 *************** *** 2130,2132 **** --- 2130,2175 ---- return result; } + + /* Detach the default sequence and the relation */ + + void + RemoveSequenceDefault(Oid relid, AttrNumber attnum, + DropBehavior behavior) + { + Relation attrdef_rel; + ScanKeyData scankeys[2]; + SysScanDesc scan; + HeapTuple tuple; + + attrdef_rel = heap_open(AttrDefaultRelationId, RowExclusiveLock); + + ScanKeyInit(&scankeys[0], + Anum_pg_attrdef_adrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(relid)); + ScanKeyInit(&scankeys[1], + Anum_pg_attrdef_adnum, + BTEqualStrategyNumber, F_INT2EQ, + Int16GetDatum(attnum)); + + scan = systable_beginscan(attrdef_rel, AttrDefaultIndexId, true, + SnapshotNow, 2, scankeys); + + /* There should be at most one matching tuple, but we loop anyway */ + while (HeapTupleIsValid(tuple = systable_getnext(scan))) + { + ObjectAddress object; + + object.classId = AttrDefaultRelationId; + object.objectId = HeapTupleGetOid(tuple); + object.objectSubId = 0; + + performSequenceDefaultDeletion(&object, behavior, 0); + + } + + systable_endscan(scan); + heap_close(attrdef_rel, RowExclusiveLock); + + } *** ./src/backend/commands/tablecmds.c.orig Mon Apr 10 13:48:00 2006 --- ./src/backend/commands/tablecmds.c Mon Apr 10 13:50:54 2006 *************** *** 3362,3367 **** --- 3362,3368 ---- * safety, but at present we do not expect anything to depend on the * default. */ + RemoveSequenceDefault(RelationGetRelid(rel), attnum, DROP_RESTRICT); RemoveAttrDefault(RelationGetRelid(rel), attnum, DROP_RESTRICT, false); if (newDefault) *** ./src/include/catalog/dependency.h.orig Mon Apr 10 13:59:00 2006 --- ./src/include/catalog/dependency.h Mon Apr 10 14:01:16 2006 *************** *** 207,210 **** --- 207,214 ---- extern void shdepReassignOwned(List *relids, Oid newrole); + extern void performSequenceDefaultDeletion(const ObjectAddress *object, + DropBehavior behavior, int deleteFlag); + + #endif /* DEPENDENCY_H */ *** ./src/include/catalog/heap.h.orig Mon Apr 10 13:59:47 2006 --- ./src/include/catalog/heap.h Mon Apr 10 14:01:23 2006 *************** *** 97,100 **** --- 97,103 ---- extern void CheckAttributeType(const char *attname, Oid atttypid); + extern void RemoveSequenceDefault(Oid relid, AttrNumber attnum, + DropBehavior behavior); + #endif /* HEAP_H */
Dhanaraj M - Sun Microsystems <dhanaraj.m@mail-apac.sun.com> writes: > I fixed the above bug. I attach the patch here. Please review and > acknowledge me. > Bug details > ======== > BUG #2073: Can't drop sequence when created via SERIAL column That isn't a bug, and this "fix" is not appropriate. See eg Bruce's response to that bug report: http://archives.postgresql.org/pgsql-bugs/2005-11/msg00304.php I speculated a bit ago (can't find it in the archives at the moment) that we should abandon the hidden dependency altogether, and go back to having SERIAL just create the sequence and the default expression. Another idea that might be worth exploring is to link the sequence to the default expression rather than to the table column itself. But randomly dropping the dependency is not the answer. regards, tom lane
Pl. look at the following code, which is taken from alter_table.sql (regression test) ========================================================================= mydb=# create table anothertab (atcol1 serial8, atcol2 boolean, constraint anothertab_chk check (atcol1 <= 3)); NOTICE: CREATE TABLE will create implicit sequence "anothertab_atcol1_seq" for serial column "anothertab.atcol1" CREATE TABLE mydb=# alter table anothertab alter column atcol1 drop default; ALTER TABLE mydb=# \d List of relations Schema | Name | Type | Owner --------+-----------------------+----------+---------- public | anothertab | table | dm199272 public | anothertab_atcol1_seq | sequence | dm199272 (2 rows) mydb=# drop sequence anothertab_atcol1_seq; ERROR: cannot drop sequence anothertab_atcol1_seq because table anothertab column atcol1 requires it HINT: You may drop table anothertab column atcol1 instead. ========================================================================= Please tell me whether statement-2 is valid or not (as you say that the default sequence should not be changed). Or the default seq. can be dropped and cant be changed. I like to send you the appropriate patch and waiting for your reply. Thanks Dhanaraj Tom Lane wrote: >Dhanaraj M - Sun Microsystems <dhanaraj.m@mail-apac.sun.com> writes: > > >>I fixed the above bug. I attach the patch here. Please review and >>acknowledge me. >> >> > > > >>Bug details >>======== >>BUG #2073: Can't drop sequence when created via SERIAL column >> >> > >That isn't a bug, and this "fix" is not appropriate. See eg Bruce's >response to that bug report: >http://archives.postgresql.org/pgsql-bugs/2005-11/msg00304.php > >I speculated a bit ago (can't find it in the archives at the moment) >that we should abandon the hidden dependency altogether, and go back to >having SERIAL just create the sequence and the default expression. >Another idea that might be worth exploring is to link the sequence to >the default expression rather than to the table column itself. But >randomly dropping the dependency is not the answer. > > regards, tom lane > >---------------------------(end of broadcast)--------------------------- >TIP 3: Have you checked our extensive FAQ? > > http://www.postgresql.org/docs/faq > >