Thread: 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)
From
Dhanaraj M - Sun Microsystems
Date:
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
>
>