Patchers,
Here is a patch regarding ALTER TABLE ... OWNER and the sequences that
were created by a SERIAL column.
It uses pg_depend to find SERIAL sequences, and recurses the ChangeOwner
to them. Additionally, it forbids directly changing the owner of a
SERIAL sequence (the error message of this last action needs to be
perfected).
I also added some regression tests for ALTER TABLE ... OWNER, because
previously there wasn't any.
According to my testing, this fixes the issue with pg_dump not restoring
the ownership of SERIAL sequences and the annoyance of having to alter
the sequence manually (the principle of least surprise indicates that
the sequence should be altered automatically).
Please review.
--
Alvaro Herrera (<alvherre[a]dcc.uchile.cl>)
"Now I have my system running, not a byte was off the shelf;
It rarely breaks and when it does I fix the code myself.
It's stable, clean and elegant, and lightning fast as well,
And it doesn't cost a nickel, so Bill Gates can go to hell."
Index: src/backend/commands/tablecmds.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql-server/src/backend/commands/tablecmds.c,v
retrieving revision 1.132
diff -c -r1.132 tablecmds.c
*** src/backend/commands/tablecmds.c 16 Sep 2004 16:58:28 -0000 1.132
--- src/backend/commands/tablecmds.c 22 Sep 2004 18:57:43 -0000
***************
*** 236,242 ****
const char *colName, TypeName *typename);
static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab);
static void ATPostAlterTypeParse(char *cmd, List **wqueue);
! static void ATExecChangeOwner(Oid relationOid, int32 newOwnerSysId);
static void ATExecClusterOn(Relation rel, const char *indexName);
static void ATExecDropCluster(Relation rel);
static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
--- 236,244 ----
const char *colName, TypeName *typename);
static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab);
static void ATPostAlterTypeParse(char *cmd, List **wqueue);
! static void ATExecChangeOwner(Oid relationOid, int32 newOwnerSysId, bool recursing);
! static void change_owner_recurse_to_sequences(Oid relationOid, int newOwnerSysId);
! static void change_owner_check_serial_sequence(Oid relationOid);
static void ATExecClusterOn(Relation rel, const char *indexName);
static void ATExecDropCluster(Relation rel);
static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
***************
*** 2114,2120 ****
break;
case AT_ChangeOwner: /* ALTER OWNER */
/* get_usesysid raises an error if no such user */
! ATExecChangeOwner(RelationGetRelid(rel),
get_usesysid(cmd->name));
break;
case AT_ClusterOn: /* CLUSTER ON */
ATExecClusterOn(rel, cmd->name);
--- 2116,2122 ----
break;
case AT_ChangeOwner: /* ALTER OWNER */
/* get_usesysid raises an error if no such user */
! ATExecChangeOwner(RelationGetRelid(rel),
get_usesysid(cmd->name), false);
break;
case AT_ClusterOn: /* CLUSTER ON */
ATExecClusterOn(rel, cmd->name);
***************
*** 5114,5128 ****
* ALTER TABLE OWNER
*/
static void
! ATExecChangeOwner(Oid relationOid, int32 newOwnerSysId)
{
Relation target_rel;
Relation class_rel;
HeapTuple tuple;
Form_pg_class tuple_class;
! /* Get exclusive lock till end of transaction on the target table */
! /* Use relation_open here so that we work on indexes... */
target_rel = relation_open(relationOid, AccessExclusiveLock);
/* Get its pg_class tuple, too */
--- 5116,5132 ----
* ALTER TABLE OWNER
*/
static void
! ATExecChangeOwner(Oid relationOid, int32 newOwnerSysId, bool recursing)
{
Relation target_rel;
Relation class_rel;
HeapTuple tuple;
Form_pg_class tuple_class;
! /*
! * Get exclusive lock till end of transaction on the target table.
! * Use relation_open here so that we work on indexes...
! */
target_rel = relation_open(relationOid, AccessExclusiveLock);
/* Get its pg_class tuple, too */
***************
*** 5141,5150 ****
case RELKIND_RELATION:
case RELKIND_INDEX:
case RELKIND_VIEW:
- case RELKIND_SEQUENCE:
case RELKIND_TOASTVALUE:
/* ok to change owner */
break;
default:
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
--- 5145,5161 ----
case RELKIND_RELATION:
case RELKIND_INDEX:
case RELKIND_VIEW:
case RELKIND_TOASTVALUE:
/* ok to change owner */
break;
+ case RELKIND_SEQUENCE:
+ /*
+ * If we were invoked for a sequence, it's ok to change owner
+ * unless it's SERIAL. If we are recursing, no need to worry.
+ */
+ if (!recursing)
+ change_owner_check_serial_sequence(relationOid);
+ break;
default:
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
***************
*** 5202,5209 ****
/*
* If we are operating on a table, also change the ownership of
! * any indexes that belong to the table, as well as the table's
! * toast table (if it has one)
*/
if (tuple_class->relkind == RELKIND_RELATION ||
tuple_class->relkind == RELKIND_TOASTVALUE)
--- 5213,5220 ----
/*
* If we are operating on a table, also change the ownership of
! * any indexes and sequences that belong to the table, as well as
! * the table's toast table (if it has one)
*/
if (tuple_class->relkind == RELKIND_RELATION ||
tuple_class->relkind == RELKIND_TOASTVALUE)
***************
*** 5216,5222 ****
/* For each index, recursively change its ownership */
foreach(i, index_oid_list)
! ATExecChangeOwner(lfirst_oid(i), newOwnerSysId);
list_free(index_oid_list);
}
--- 5227,5233 ----
/* For each index, recursively change its ownership */
foreach(i, index_oid_list)
! ATExecChangeOwner(lfirst_oid(i), newOwnerSysId, true);
list_free(index_oid_list);
}
***************
*** 5225,5231 ****
{
/* If it has a toast table, recurse to change its ownership */
if (tuple_class->reltoastrelid != InvalidOid)
! ATExecChangeOwner(tuple_class->reltoastrelid,
newOwnerSysId);
}
}
--- 5236,5245 ----
{
/* If it has a toast table, recurse to change its ownership */
if (tuple_class->reltoastrelid != InvalidOid)
! ATExecChangeOwner(tuple_class->reltoastrelid,
newOwnerSysId, true);
!
! /* If it has dependant sequences, recurse to them */
! change_owner_recurse_to_sequences(relationOid, newOwnerSysId);
}
}
***************
*** 5235,5240 ****
--- 5249,5362 ----
}
/*
+ * change_owner_check_serial_sequence
+ *
+ * Helper function for ATExecChangeOwner. Examines pg_depend to see
+ * if the sequence is wedged to a SERIAL column. Error out if so.
+ */
+ static void
+ change_owner_check_serial_sequence(Oid relationOid)
+ {
+ Relation depRel;
+ HeapTuple tup;
+ SysScanDesc scan;
+ ScanKeyData key[2];
+
+ depRel = heap_openr(DependRelationName, RowExclusiveLock);
+
+ ScanKeyInit(&key[0],
+ Anum_pg_depend_classid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(RelOid_pg_class));
+ ScanKeyInit(&key[1],
+ Anum_pg_depend_objid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(relationOid));
+
+ scan = systable_beginscan(depRel, DependDependerIndex, true,
+ SnapshotNow, 2, key);
+
+ while (HeapTupleIsValid(tup = systable_getnext(scan)))
+ {
+ Form_pg_depend depForm = (Form_pg_depend) GETSTRUCT(tup);
+
+ /*
+ * We assume that a SERIAL sequence will have an entry
+ * to a pg_class object, and that a manually created
+ * sequence won't.
+ */
+ if (depForm->refclassid == RelOid_pg_class)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("can't change ownership of
automatically-created sequences")));
+ }
+
+ systable_endscan(scan);
+ heap_close(depRel, RowExclusiveLock);
+ }
+
+ /*
+ * change_owner_recurse_to_sequences
+ *
+ * Helper function for ATExecChangeOwner. Examines pg_depend searching
+ * for sequences that are referenced from a table, and changes their
+ * owner.
+ */
+ static void
+ change_owner_recurse_to_sequences(Oid relId, int newOwnerSysId)
+ {
+ Relation depRel;
+ SysScanDesc scan;
+ ScanKeyData key[2];
+ HeapTuple tup;
+
+ depRel = heap_openr(DependRelationName, RowExclusiveLock);
+
+ ScanKeyInit(&key[0],
+ Anum_pg_depend_refclassid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(RelOid_pg_class));
+ ScanKeyInit(&key[1],
+ Anum_pg_depend_refobjid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(relId));
+
+ scan = systable_beginscan(depRel, DependReferenceIndex, true,
+ SnapshotNow, 2, key);
+
+ while (HeapTupleIsValid(tup = systable_getnext(scan)))
+ {
+ Form_pg_depend depForm = (Form_pg_depend) GETSTRUCT(tup);
+ Relation seqRel;
+
+ /* skip non-relation dependencies */
+ if (depForm->classid != RelOid_pg_class)
+ continue;
+
+ /*
+ * The relation could be an index, so don't use heap_open.
+ */
+ seqRel = relation_open(depForm->objid, AccessExclusiveLock);
+
+ /* skip non-sequence relations */
+ if (RelationGetForm(seqRel)->relkind != RELKIND_SEQUENCE)
+ {
+ /* No need to keep the lock */
+ relation_close(seqRel, AccessExclusiveLock);
+ continue;
+ }
+
+ /* We don't need to close the sequence while we alter it. */
+ ATExecChangeOwner(depForm->objid, newOwnerSysId, true);
+
+ /* Now we can close it. Keep the lock till end of transaction. */
+ relation_close(seqRel, NoLock);
+ }
+
+ systable_endscan(scan);
+ }
+
+ /*
* ALTER TABLE CLUSTER ON
*
* The only thing we have to do is to change the indisclustered bits.
Index: src/test/regress/expected/alter_table.out
===================================================================
RCS file: /home/alvherre/cvs/pgsql-server/src/test/regress/expected/alter_table.out,v
retrieving revision 1.85
diff -c -r1.85 alter_table.out
*** src/test/regress/expected/alter_table.out 28 Aug 2004 22:06:04 -0000 1.85
--- src/test/regress/expected/alter_table.out 22 Sep 2004 18:41:17 -0000
***************
*** 1235,1237 ****
--- 1235,1295 ----
(3 rows)
drop table another;
+ -- alter table ... owner to
+ create user ownertest with sysid 42;
+ create table ownertest (a serial primary key, c text, b serial);
+ NOTICE: CREATE TABLE will create implicit sequence "ownertest_a_seq" for serial
column "ownertest.a"
+ NOTICE: CREATE TABLE will create implicit sequence "ownertest_b_seq" for serial
column "ownertest.b"
+ NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "ownertest_pkey" for
table "ownertest"
+ create index ownertest_idx on ownertest(c);
+ -- this catches indexes and sequences
+ select relname, relowner from pg_class where relname like 'ownert%';
+ relname | relowner
+ -----------------+----------
+ ownertest_a_seq | 1
+ ownertest_b_seq | 1
+ ownertest_pkey | 1
+ ownertest | 1
+ ownertest_idx | 1
+ (5 rows)
+
+ -- this catches the toast table and its index.
+ -- Don't get the relname because it will change across runs ...
+ select a.relowner as "toast owner", b.relname, c.relowner as "toast idx owner"
+ from pg_class a join pg_class b on (a.oid = b.reltoastrelid)
+ join pg_class c on (a.reltoastidxid = c.oid)
+ where b.oid = 'ownertest'::regclass;
+ toast owner | relname | toast idx owner
+ -------------+-----------+-----------------
+ 1 | ownertest | 1
+ (1 row)
+
+ alter table ownertest owner to ownertest;
+ select relname, relowner from pg_class where relname like 'ownert%';
+ relname | relowner
+ -----------------+----------
+ ownertest | 42
+ ownertest_pkey | 42
+ ownertest_idx | 42
+ ownertest_a_seq | 42
+ ownertest_b_seq | 42
+ (5 rows)
+
+ select a.relowner as "toast owner", b.relname, c.relowner as "toast idx owner"
+ from pg_class a join pg_class b on (a.oid = b.reltoastrelid)
+ join pg_class c on (a.reltoastidxid = c.oid)
+ where b.oid = 'ownertest'::regclass;
+ toast owner | relname | toast idx owner
+ -------------+-----------+-----------------
+ 42 | ownertest | 42
+ (1 row)
+
+ -- fail: can't alter a SERIAL sequence
+ alter table ownertest_a_seq owner to ownertest;
+ ERROR: can't change ownership of automatically-created sequences
+ create sequence ownertest_seq;
+ -- pass: alter a normal sequence is allowed
+ alter table ownertest_seq owner to ownertest;
+ drop sequence ownertest_seq;
+ drop table ownertest;
+ drop user ownertest;
Index: src/test/regress/sql/alter_table.sql
===================================================================
RCS file: /home/alvherre/cvs/pgsql-server/src/test/regress/sql/alter_table.sql,v
retrieving revision 1.47
diff -c -r1.47 alter_table.sql
*** src/test/regress/sql/alter_table.sql 28 Aug 2004 22:06:04 -0000 1.47
--- src/test/regress/sql/alter_table.sql 22 Sep 2004 18:40:14 -0000
***************
*** 975,977 ****
--- 975,1004 ----
select * from another;
drop table another;
+
+ -- alter table ... owner to
+ create user ownertest with sysid 42;
+ create table ownertest (a serial primary key, c text, b serial);
+ create index ownertest_idx on ownertest(c);
+ -- this catches indexes and sequences
+ select relname, relowner from pg_class where relname like 'ownert%';
+ -- this catches the toast table and its index.
+ -- Don't get the relname because it will change across runs ...
+ select a.relowner as "toast owner", b.relname, c.relowner as "toast idx owner"
+ from pg_class a join pg_class b on (a.oid = b.reltoastrelid)
+ join pg_class c on (a.reltoastidxid = c.oid)
+ where b.oid = 'ownertest'::regclass;
+ alter table ownertest owner to ownertest;
+ select relname, relowner from pg_class where relname like 'ownert%';
+ select a.relowner as "toast owner", b.relname, c.relowner as "toast idx owner"
+ from pg_class a join pg_class b on (a.oid = b.reltoastrelid)
+ join pg_class c on (a.reltoastidxid = c.oid)
+ where b.oid = 'ownertest'::regclass;
+ -- fail: can't alter a SERIAL sequence
+ alter table ownertest_a_seq owner to ownertest;
+ create sequence ownertest_seq;
+ -- pass: alter a normal sequence is allowed
+ alter table ownertest_seq owner to ownertest;
+ drop sequence ownertest_seq;
+ drop table ownertest;
+ drop user ownertest;
---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster
