From: Tom Lane Date: Fri, 2 Oct 2009 18:13:32 +0000 (+0000) Subject: Fix erroneous handling of shared dependencies (ie dependencies on roles) X-Git-Tag: REL8_1_19~16 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=595f2744b0498e5b82649242aae35553761f3081;p=thirdparty%2Fpostgresql.git Fix erroneous handling of shared dependencies (ie dependencies on roles) in CREATE OR REPLACE FUNCTION. The original code would update pg_shdepend as if a new function was being created, even if it wasn't, with two bad consequences: pg_shdepend might record the wrong owner for the function, and any dependencies for roles mentioned in the function's ACL would be lost. The fix is very easy: just don't touch pg_shdepend at all when doing a function replacement. Also update the CREATE FUNCTION reference page, which never explained exactly what changes and doesn't change in a function replacement. In passing, fix the CREATE VIEW reference page similarly; there's no code bug there, but the docs didn't say what happens. --- diff --git a/doc/src/sgml/ref/create_function.sgml b/doc/src/sgml/ref/create_function.sgml index 0b0510d878b..ecb6b918f85 100644 --- a/doc/src/sgml/ref/create_function.sgml +++ b/doc/src/sgml/ref/create_function.sgml @@ -1,5 +1,5 @@ @@ -421,6 +421,14 @@ CREATE FUNCTION foo(int, out text) ... USAGE privilege on the language. + + When CREATE OR REPLACE FUNCTION is used to replace an + existing function, the ownership and permissions of the function + do not change. All other function properties are assigned the + values specified or implied in the command. You must own the function + to replace it (this includes being a member of the owning role). + + diff --git a/doc/src/sgml/ref/create_view.sgml b/doc/src/sgml/ref/create_view.sgml index aaff51b2a09..c6a55067a18 100644 --- a/doc/src/sgml/ref/create_view.sgml +++ b/doc/src/sgml/ref/create_view.sgml @@ -1,5 +1,5 @@ @@ -150,6 +150,14 @@ CREATE VIEW vista AS SELECT text 'Hello World' AS hello; used by the view. + + When CREATE OR REPLACE VIEW is used on an + existing view, only the view's defining SELECT rule is changed. + Other view properties, including ownership, permissions, and non-SELECT + rules, remain unchanged. You must own the view + to replace it (this includes being a member of the owning role). + + diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c index de2a5bf554f..f1a75de9271 100644 --- a/src/backend/catalog/pg_proc.c +++ b/src/backend/catalog/pg_proc.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/catalog/pg_proc.c,v 1.135.2.1 2005/11/22 18:23:06 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/catalog/pg_proc.c,v 1.135.2.2 2009/10/02 18:13:32 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -84,6 +84,7 @@ ProcedureCreate(const char *procedureName, bool genericOutParam = false; bool internalInParam = false; bool internalOutParam = false; + Oid proowner = GetUserId(); Relation rel; HeapTuple tup; HeapTuple oldtup; @@ -217,7 +218,7 @@ ProcedureCreate(const char *procedureName, namestrcpy(&procname, procedureName); values[Anum_pg_proc_proname - 1] = NameGetDatum(&procname); values[Anum_pg_proc_pronamespace - 1] = ObjectIdGetDatum(procNamespace); - values[Anum_pg_proc_proowner - 1] = ObjectIdGetDatum(GetUserId()); + values[Anum_pg_proc_proowner - 1] = ObjectIdGetDatum(proowner); values[Anum_pg_proc_prolang - 1] = ObjectIdGetDatum(languageObjectId); values[Anum_pg_proc_proisagg - 1] = BoolGetDatum(isAgg); values[Anum_pg_proc_prosecdef - 1] = BoolGetDatum(security_definer); @@ -266,7 +267,7 @@ ProcedureCreate(const char *procedureName, (errcode(ERRCODE_DUPLICATE_FUNCTION), errmsg("function \"%s\" already exists with same argument types", procedureName))); - if (!pg_proc_ownercheck(HeapTupleGetOid(oldtup), GetUserId())) + if (!pg_proc_ownercheck(HeapTupleGetOid(oldtup), proowner)) aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_PROC, procedureName); @@ -320,7 +321,10 @@ ProcedureCreate(const char *procedureName, procedureName))); } - /* do not change existing ownership or permissions, either */ + /* + * Do not change existing ownership or permissions, either. Note + * dependency-update code below has to agree with this decision. + */ replaces[Anum_pg_proc_proowner - 1] = ' '; replaces[Anum_pg_proc_proacl - 1] = ' '; @@ -347,12 +351,11 @@ ProcedureCreate(const char *procedureName, /* * Create dependencies for the new function. If we are updating an * existing function, first delete any existing pg_depend entries. + * (However, since we are not changing ownership or permissions, the + * shared dependencies do *not* need to change, and we leave them alone.) */ if (is_update) - { deleteDependencyRecordsFor(ProcedureRelationId, retval); - deleteSharedDependencyRecordsFor(ProcedureRelationId, retval); - } myself.classId = ProcedureRelationId; myself.objectId = retval; @@ -386,7 +389,8 @@ ProcedureCreate(const char *procedureName, } /* dependency on owner */ - recordDependencyOnOwner(ProcedureRelationId, retval, GetUserId()); + if (!is_update) + recordDependencyOnOwner(ProcedureRelationId, retval, proowner); heap_freetuple(tup);