Thread: Patch for BUG #2073: Can't drop sequence when created via SERIAL column
Hi
I send the appropriate patch for bug #2073. This fix disallows to change the default sequence.
I ran the regression test and passed. The bug details are given below.
I am awaiting to answer for any further clarifications.
===================================================================
> 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.
Right. We have this TODO item:
* %Disallow changing default expression of a SERIAL column?
which would prevent you from changing the default expression for a
SERIAL column. So the answer is, don't do that, and in the future, we
might prevent it.
--
Bruce Momjian
==================================================================
*** ./src/backend/catalog/dependency.c.orig Wed Apr 26 10:54:40 2006
--- ./src/backend/catalog/dependency.c Wed Apr 26 12:09:01 2006
***************
*** 1931,1933 ****
--- 1931,2019 ----
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)
+ {
+ /* Dont allow to change the default sequence */
+ if(deleteFlag == 2)
+ {
+ systable_endscan(scan);
+ heap_close(depRel, RowExclusiveLock);
+ elog(ERROR, "%s is a SERIAL sequence. Can't alter the relation",
getObjectDescription(&otherObject));
+ return;
+ }
+ else /* Detach the default sequence from the relation */
+ {
+ 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 Wed Apr 26 10:59:18 2006
--- ./src/backend/catalog/heap.c Wed Apr 26 12:03:49 2006
***************
*** 2136,2138 ****
--- 2136,2185 ----
return result;
}
+
+
+ /* Detach the default sequence and the relation */
+
+ void
+ RemoveSequenceDefault(Oid relid, AttrNumber attnum,
+ DropBehavior behavior, bool flag)
+ {
+ 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;
+
+ if(flag == true) /* Detach the sequence */
+ performSequenceDefaultDeletion(&object, behavior, 0);
+ else /* Don't allow to change the default sequence */
+ performSequenceDefaultDeletion(&object, behavior, 2);
+
+ }
+
+ systable_endscan(scan);
+ heap_close(attrdef_rel, RowExclusiveLock);
+
+ }
*** ./src/backend/commands/tablecmds.c.orig Wed Apr 26 11:00:14 2006
--- ./src/backend/commands/tablecmds.c Wed Apr 26 11:57:43 2006
***************
*** 3362,3367 ****
--- 3362,3372 ----
* safety, but at present we do not expect anything to depend on the
* default.
*/
+ if (newDefault)
+ RemoveSequenceDefault(RelationGetRelid(rel), attnum, DROP_RESTRICT, false);
+ else
+ RemoveSequenceDefault(RelationGetRelid(rel), attnum, DROP_RESTRICT, true);
+
RemoveAttrDefault(RelationGetRelid(rel), attnum, DROP_RESTRICT, false);
if (newDefault)
*** ./src/include/catalog/dependency.h.orig Wed Apr 26 11:05:28 2006
--- ./src/include/catalog/dependency.h Wed Apr 26 11:06:13 2006
***************
*** 207,210 ****
--- 207,213 ----
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 Wed Apr 26 11:04:59 2006
--- ./src/include/catalog/heap.h Wed Apr 26 11:57:56 2006
***************
*** 97,100 ****
--- 97,103 ----
extern void CheckAttributeType(const char *attname, Oid atttypid);
+ extern void RemoveSequenceDefault(Oid relid, AttrNumber attnum,
+ DropBehavior behavior, bool flag);
+
#endif /* HEAP_H */
Patch applied. Thanks. --------------------------------------------------------------------------- Dhanaraj M wrote: > Hi > > I send the appropriate patch for bug #2073. This fix disallows to change the default sequence. > I ran the regression test and passed. The bug details are given below. > I am awaiting to answer for any further clarifications. > > =================================================================== > > 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. > > Right. We have this TODO item: > > * %Disallow changing default expression of a SERIAL column? > > which would prevent you from changing the default expression for a > SERIAL column. So the answer is, don't do that, and in the future, we > might prevent it. > > -- > Bruce Momjian > > ================================================================== > > > ---------------------------(end of broadcast)--------------------------- > TIP 5: don't forget to increase your free space map settings -- Bruce Momjian http://candle.pha.pa.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Patch applied. Thanks.
Has this been reviewed at all? I thought we were still discussing what
the behavior should be, let alone whether the patch is correct.
regards, tom lane
I wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> Patch applied. Thanks.
> Has this been reviewed at all? I thought we were still discussing what
> the behavior should be, let alone whether the patch is correct.
Actually, now that I look at it, this patch is definitely *not* correct,
as it does not implement any of the behaviors suggested as reasonable;
and the discussion is still ongoing in any case.
Please revert.
regards, tom lane
Tom Lane wrote: > I wrote: > > Bruce Momjian <pgman@candle.pha.pa.us> writes: > >> Patch applied. Thanks. > > > Has this been reviewed at all? I thought we were still discussing what > > the behavior should be, let alone whether the patch is correct. > > Actually, now that I look at it, this patch is definitely *not* correct, > as it does not implement any of the behaviors suggested as reasonable; > and the discussion is still ongoing in any case. > > Please revert. Done. -- Bruce Momjian http://candle.pha.pa.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Patch applied. Thanks. --------------------------------------------------------------------------- Dhanaraj M wrote: > Hi > > I send the appropriate patch for bug #2073. This fix disallows to change the default sequence. > I ran the regression test and passed. The bug details are given below. > I am awaiting to answer for any further clarifications. > > =================================================================== > > 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. > > Right. We have this TODO item: > > * %Disallow changing default expression of a SERIAL column? > > which would prevent you from changing the default expression for a > SERIAL column. So the answer is, don't do that, and in the future, we > might prevent it. > > -- > Bruce Momjian > > ================================================================== > > > ---------------------------(end of broadcast)--------------------------- > TIP 5: don't forget to increase your free space map settings -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +