Patch for bug #2073 (Can't drop sequence when created via SERIAL column) - Mailing list pgsql-patches

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 */