]> granicus.if.org Git - postgresql/commitdiff
ALTER TABLE OWNER must change the ownership of the table's rowtype too.
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 4 Aug 2005 01:09:29 +0000 (01:09 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 4 Aug 2005 01:09:29 +0000 (01:09 +0000)
This was not especially critical before, but it is now that we track
ownership dependencies --- the dependency for the rowtype *must* shift
to the new owner.  Spotted by Bernd Helmle.
Also fix a problem introduced by recent change to allow non-superusers
to do ALTER OWNER in some cases: if the table had a toast table, ALTER
OWNER failed *even for superusers*, because the test being applied would
conclude that the new would-be owner had no create rights on pg_toast.
A side-effect of the fix is to disallow changing the ownership of indexes
or toast tables separately from their parent table, which seems a good
idea on the whole.

doc/src/sgml/ref/alter_table.sgml
src/backend/commands/tablecmds.c
src/backend/commands/typecmds.c
src/include/commands/typecmds.h
src/test/regress/expected/dependency.out
src/test/regress/sql/dependency.sql

index 26dabbb79ecbd3f328a1a4a8a4878acc963800b8..8e1d29842494fd98e159968942ec5492f5865493 100644 (file)
@@ -1,5 +1,5 @@
 <!--
-$PostgreSQL: pgsql/doc/src/sgml/ref/alter_table.sgml,v 1.78 2005/08/01 16:11:14 tgl Exp $
+$PostgreSQL: pgsql/doc/src/sgml/ref/alter_table.sgml,v 1.79 2005/08/04 01:09:27 tgl Exp $
 PostgreSQL documentation
 -->
 
@@ -235,7 +235,7 @@ where <replaceable class="PARAMETER">action</replaceable> is one of:
     <term><literal>OWNER</literal></term>
     <listitem>
      <para>
-      This form changes the owner of the table, index, sequence, or view to the
+      This form changes the owner of the table, sequence, or view to the
       specified user.
      </para>
     </listitem>
index 7d9a73917ddd2565792d9509b002b0f8ed0f975b..aaf9a2ce7435d18240af257109ed7d1c82352eee 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.165 2005/08/01 04:03:55 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.166 2005/08/04 01:09:28 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -238,7 +238,7 @@ static void ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
                                          const char *colName, TypeName *typename);
 static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab);
 static void ATPostAlterTypeParse(char *cmd, List **wqueue);
-static void ATExecChangeOwner(Oid relationOid, Oid newOwnerId);
+static void ATExecChangeOwner(Oid relationOid, Oid newOwnerId, bool recursing);
 static void change_owner_recurse_to_sequences(Oid relationOid,
                                                                                          Oid newOwnerId);
 static void ATExecClusterOn(Relation rel, const char *indexName);
@@ -2141,7 +2141,8 @@ ATExecCmd(AlteredTableInfo *tab, Relation rel, AlterTableCmd *cmd)
                        break;
                case AT_ChangeOwner:    /* ALTER OWNER */
                        ATExecChangeOwner(RelationGetRelid(rel),
-                                                         get_roleid_checked(cmd->name));
+                                                         get_roleid_checked(cmd->name),
+                                                         false);
                        break;
                case AT_ClusterOn:              /* CLUSTER ON */
                        ATExecClusterOn(rel, cmd->name);
@@ -5238,9 +5239,15 @@ ATPostAlterTypeParse(char *cmd, List **wqueue)
 
 /*
  * ALTER TABLE OWNER
+ *
+ * recursing is true if we are recursing from a table to its indexes or
+ * toast table.  We don't allow the ownership of those things to be
+ * changed separately from the parent table.  Also, we can skip permission
+ * checks (this is necessary not just an optimization, else we'd fail to
+ * handle toast tables properly).
  */
 static void
-ATExecChangeOwner(Oid relationOid, Oid newOwnerId)
+ATExecChangeOwner(Oid relationOid, Oid newOwnerId, bool recursing)
 {
        Relation        target_rel;
        Relation        class_rel;
@@ -5267,16 +5274,19 @@ ATExecChangeOwner(Oid relationOid, Oid newOwnerId)
        switch (tuple_class->relkind)
        {
                case RELKIND_RELATION:
-               case RELKIND_INDEX:
                case RELKIND_VIEW:
                case RELKIND_SEQUENCE:
-               case RELKIND_TOASTVALUE:
                        /* ok to change owner */
                        break;
+               case RELKIND_INDEX:
+               case RELKIND_TOASTVALUE:
+                       if (recursing)
+                               break;
+                       /* FALL THRU */
                default:
                        ereport(ERROR,
                                        (errcode(ERRCODE_WRONG_OBJECT_TYPE),
-                                        errmsg("\"%s\" is not a table, TOAST table, index, view, or sequence",
+                                        errmsg("\"%s\" is not a table, view, or sequence",
                                                        NameStr(tuple_class->relname))));
        }
 
@@ -5293,23 +5303,28 @@ ATExecChangeOwner(Oid relationOid, Oid newOwnerId)
                Datum           aclDatum;
                bool            isNull;
                HeapTuple       newtuple;
-               Oid                 namespaceOid = tuple_class->relnamespace;
-               AclResult       aclresult;
-
-               /* Otherwise, must be owner of the existing object */
-               if (!pg_class_ownercheck(relationOid,GetUserId()))
-                       aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
-                                                  RelationGetRelationName(target_rel));
 
-               /* Must be able to become new owner */
-               check_is_member_of_role(GetUserId(), newOwnerId);
-
-               /* New owner must have CREATE privilege on namespace */
-               aclresult = pg_namespace_aclcheck(namespaceOid, newOwnerId,
-                                                                                 ACL_CREATE);
-               if (aclresult != ACLCHECK_OK)
-                       aclcheck_error(aclresult, ACL_KIND_NAMESPACE,
-                                                  get_namespace_name(namespaceOid));
+               /* skip permission checks when recursing to index or toast table */
+               if (!recursing)
+               {
+                       Oid                 namespaceOid = tuple_class->relnamespace;
+                       AclResult       aclresult;
+
+                       /* Otherwise, must be owner of the existing object */
+                       if (!pg_class_ownercheck(relationOid,GetUserId()))
+                               aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
+                                                          RelationGetRelationName(target_rel));
+
+                       /* Must be able to become new owner */
+                       check_is_member_of_role(GetUserId(), newOwnerId);
+
+                       /* New owner must have CREATE privilege on namespace */
+                       aclresult = pg_namespace_aclcheck(namespaceOid, newOwnerId,
+                                                                                         ACL_CREATE);
+                       if (aclresult != ACLCHECK_OK)
+                               aclcheck_error(aclresult, ACL_KIND_NAMESPACE,
+                                                          get_namespace_name(namespaceOid));
+               }
 
                memset(repl_null, ' ', sizeof(repl_null));
                memset(repl_repl, ' ', sizeof(repl_repl));
@@ -5342,6 +5357,12 @@ ATExecChangeOwner(Oid relationOid, Oid newOwnerId)
                /* Update owner dependency reference */
                changeDependencyOnOwner(RelationRelationId, relationOid, newOwnerId);
 
+               /*
+                * Also change the ownership of the table's rowtype, if it has one
+                */
+               if (tuple_class->relkind != RELKIND_INDEX)
+                       AlterTypeOwnerInternal(tuple_class->reltype, newOwnerId);
+
                /*
                 * If we are operating on a table, also change the ownership of
                 * any indexes and sequences that belong to the table, as well as
@@ -5358,7 +5379,7 @@ ATExecChangeOwner(Oid relationOid, Oid newOwnerId)
 
                        /* For each index, recursively change its ownership */
                        foreach(i, index_oid_list)
-                               ATExecChangeOwner(lfirst_oid(i), newOwnerId);
+                               ATExecChangeOwner(lfirst_oid(i), newOwnerId, true);
 
                        list_free(index_oid_list);
                }
@@ -5367,7 +5388,8 @@ ATExecChangeOwner(Oid relationOid, Oid newOwnerId)
                {
                        /* If it has a toast table, recurse to change its ownership */
                        if (tuple_class->reltoastrelid != InvalidOid)
-                               ATExecChangeOwner(tuple_class->reltoastrelid, newOwnerId);
+                               ATExecChangeOwner(tuple_class->reltoastrelid, newOwnerId,
+                                                                 true);
 
                        /* If it has dependent sequences, recurse to change them too */
                        change_owner_recurse_to_sequences(relationOid, newOwnerId);
@@ -5437,7 +5459,7 @@ change_owner_recurse_to_sequences(Oid relationOid, Oid newOwnerId)
                }
 
                /* We don't need to close the sequence while we alter it. */
-               ATExecChangeOwner(depForm->objid, newOwnerId);
+               ATExecChangeOwner(depForm->objid, newOwnerId, false);
 
                /* Now we can close it.  Keep the lock till end of transaction. */
                relation_close(seqRel, NoLock);
index 80d394b29339c92f515c6907f93b16717795ac3a..31e43cd4281d12536ae91c87bbe2c12bbfb26b21 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.77 2005/08/01 04:03:55 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.78 2005/08/04 01:09:28 tgl Exp $
  *
  * DESCRIPTION
  *       The "DefineFoo" routines take the parse tree and pick out the
@@ -2057,7 +2057,8 @@ AlterTypeOwner(List *names, Oid newOwnerId)
         * free-standing composite type, and not a table's underlying type. We
         * want people to use ALTER TABLE not ALTER TYPE for that case.
         */
-       if (typTup->typtype == 'c' && get_rel_relkind(typTup->typrelid) != 'c')
+       if (typTup->typtype == 'c' &&
+               get_rel_relkind(typTup->typrelid) != RELKIND_COMPOSITE_TYPE)
                ereport(ERROR,
                                (errcode(ERRCODE_WRONG_OBJECT_TYPE),
                                 errmsg("\"%s\" is a table's row type",
@@ -2102,6 +2103,45 @@ AlterTypeOwner(List *names, Oid newOwnerId)
        heap_close(rel, RowExclusiveLock);
 }
 
+/*
+ * AlterTypeOwnerInternal - change type owner unconditionally
+ *
+ * This is currently only used to propagate ALTER TABLE OWNER to the
+ * table's rowtype.  It assumes the caller has done all needed checks.
+ */
+void
+AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId)
+{
+       Relation        rel;
+       HeapTuple       tup;
+       Form_pg_type typTup;
+
+       rel = heap_open(TypeRelationId, RowExclusiveLock);
+
+       tup = SearchSysCacheCopy(TYPEOID,
+                                                        ObjectIdGetDatum(typeOid),
+                                                        0, 0, 0);
+       if (!HeapTupleIsValid(tup))
+               elog(ERROR, "cache lookup failed for type %u", typeOid);
+       typTup = (Form_pg_type) GETSTRUCT(tup);
+
+       /*
+        * Modify the owner --- okay to scribble on typTup because it's a
+        * copy
+        */
+       typTup->typowner = newOwnerId;
+
+       simple_heap_update(rel, &tup->t_self, tup);
+
+       CatalogUpdateIndexes(rel, tup);
+
+       /* Update owner dependency reference */
+       changeDependencyOnOwner(TypeRelationId, typeOid, newOwnerId);
+
+       /* Clean up */
+       heap_close(rel, RowExclusiveLock);
+}
+
 /*
  * Execute ALTER TYPE SET SCHEMA
  */
index a070a27a29213f4df395ab4d8a95004ae76200ed..d53cf672a6553a3b8ca36eb9392e96f2199d0113 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/commands/typecmds.h,v 1.12 2005/08/01 04:03:58 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/commands/typecmds.h,v 1.13 2005/08/04 01:09:29 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -35,6 +35,7 @@ extern void AlterDomainDropConstraint(List *names, const char *constrName,
 extern List *GetDomainConstraints(Oid typeOid);
 
 extern void AlterTypeOwner(List *names, Oid newOwnerId);
+extern void AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId);
 extern void AlterTypeNamespace(List *names, const char *newschema);
 extern void AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
                                                                           bool errorOnTableType);
index 4ee3e8b6a8ff23c56c1a53817987c8cc1248d7f9..2c31e581bfe84353c63dc1dbf2ec2d0269992f06 100644 (file)
@@ -5,7 +5,9 @@ CREATE USER regression_user;
 CREATE USER regression_user2;
 CREATE USER regression_user3;
 CREATE GROUP regression_group;
-CREATE TABLE deptest ();
+CREATE TABLE deptest (f1 serial primary key, f2 text);
+NOTICE:  CREATE TABLE will create implicit sequence "deptest_f1_seq" for serial column "deptest.f1"
+NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "deptest_pkey" for table "deptest"
 GRANT SELECT ON TABLE deptest TO GROUP regression_group;
 GRANT ALL ON TABLE deptest TO regression_user, regression_user2;
 -- can't drop neither because they have privileges somewhere
@@ -30,10 +32,12 @@ DROP USER regression_user;
 REVOKE ALL ON deptest FROM regression_user2;
 DROP USER regression_user2;
 -- can't drop the owner of an object
+-- the error message detail here would include a pg_toast_nnn name that
+-- is not constant, so suppress it
+\set VERBOSITY terse
 ALTER TABLE deptest OWNER TO regression_user3;
 DROP USER regression_user3;
 ERROR:  role "regression_user3" cannot be dropped because some objects depend on it
-DETAIL:  owner of table deptest
 -- if we drop the object, we can drop the user too
 DROP TABLE deptest;
 DROP USER regression_user3;
index 6d52b62dee188037b4f4d578cb7f5490712bd36f..3e4a232ea716654908b9e0d9eb1f50c250c92644 100644 (file)
@@ -7,7 +7,7 @@ CREATE USER regression_user2;
 CREATE USER regression_user3;
 CREATE GROUP regression_group;
 
-CREATE TABLE deptest ();
+CREATE TABLE deptest (f1 serial primary key, f2 text);
 
 GRANT SELECT ON TABLE deptest TO GROUP regression_group;
 GRANT ALL ON TABLE deptest TO regression_user, regression_user2;
@@ -33,6 +33,9 @@ REVOKE ALL ON deptest FROM regression_user2;
 DROP USER regression_user2;
 
 -- can't drop the owner of an object
+-- the error message detail here would include a pg_toast_nnn name that
+-- is not constant, so suppress it
+\set VERBOSITY terse
 ALTER TABLE deptest OWNER TO regression_user3;
 DROP USER regression_user3;