ALTER TABLE OWNER must change the ownership of the table's rowtype too.
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.
This commit is contained in:
Tom Lane
parent
e48b28b688
commit
33f5bf9700
@ -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>
|
||||
|
||||
@ -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));
|
||||
/* skip permission checks when recursing to index or toast table */
|
||||
if (!recursing)
|
||||
{
|
||||
Oid namespaceOid = tuple_class->relnamespace;
|
||||
AclResult aclresult;
|
||||
|
||||
/* Must be able to become new owner */
|
||||
check_is_member_of_role(GetUserId(), newOwnerId);
|
||||
/* 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));
|
||||
|
||||
/* 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));
|
||||
/* 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);
|
||||
|
||||
@ -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
|
||||
*/
|
||||
|
||||
@ -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);
|
||||
|
||||
@ -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;
|
||||
|
||||
@ -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;
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user