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 76358a8985d249228d8b10e55df8dcb502428e94..81248300067d44a70d1441bcd633fe1c4146285b 100644 (file)
--- a/doc/src/sgml/ref/create_function.sgml
+++ b/doc/src/sgml/ref/create_function.sgml
@@ -562,6 +562,14 @@ CREATE FUNCTION foo(int, int default 42) ...
     <literal>USAGE</literal> privilege on the language.
    </para>
 
+   <para>
+    When <command>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).
+   </para>
+
  </refsect1>
 
  <refsect1 id="sql-createfunction-examples">

diff --git a/doc/src/sgml/ref/create_view.sgml b/doc/src/sgml/ref/create_view.sgml
index 2d485c33eaf4ce34b982e8887e4090d254c3a446..998bea15ec28626910ce78abb38fc6393079f569 100644 (file)
--- a/doc/src/sgml/ref/create_view.sgml
+++ b/doc/src/sgml/ref/create_view.sgml
@@ -149,6 +149,14 @@ CREATE VIEW vista AS SELECT text 'Hello World' AS hello;
     used by the view.
    </para>
 
+   <para>
+    When <command>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).
+   </para>
+
  </refsect1>
 
  <refsect1>

diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 342381e3416b026b801bf1c282e272c5cf69e4fb..d9e1e21a1704709d11192a53920d72bc6a8d85ff 100644 (file)
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -89,6 +89,7 @@ ProcedureCreate(const char *procedureName,
        bool            internalInParam = false;
        bool            internalOutParam = false;
        Oid                     variadicType = InvalidOid;
+       Oid                     proowner = GetUserId();
        Relation        rel;
        HeapTuple       tup;
        HeapTuple       oldtup;
@@ -290,7 +291,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_procost - 1] = Float4GetDatum(procost);
        values[Anum_pg_proc_prorows - 1] = Float4GetDatum(prorows);
@@ -353,7 +354,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);
 
@@ -471,7 +472,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] = false;
                replaces[Anum_pg_proc_proacl - 1] = false;
 
@@ -498,12 +502,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, 0);
-       }
 
        myself.classId = ProcedureRelationId;
        myself.objectId = retval;
@@ -537,7 +540,8 @@ ProcedureCreate(const char *procedureName,
        }
 
        /* dependency on owner */
-       recordDependencyOnOwner(ProcedureRelationId, retval, GetUserId());
+       if (!is_update)
+               recordDependencyOnOwner(ProcedureRelationId, retval, proowner);
 
        heap_freetuple(tup);