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.

1 files changed, 12 insertions, 8 deletions

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);