Fix erroneous handling of shared dependencies (ie dependencies on roles)
authorTom Lane
Fri, 2 Oct 2009 18:13:04 +0000 (18:13 +0000)
committerTom Lane
Fri, 2 Oct 2009 18:13:04 +0000 (18:13 +0000)
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.

doc/src/sgml/ref/create_function.sgml
doc/src/sgml/ref/create_view.sgml
src/backend/catalog/pg_proc.c

index 5b5cda3a8172e4a68dab96426f543f8954714f00..0843a175638a4dd0fd80ea455c1940abf5495c70 100644 (file)
@@ -1,5 +1,5 @@
 
 
 
@@ -562,6 +562,14 @@ CREATE FUNCTION foo(int, int default 42) ...
     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).
+   
+
  
 
  
index 1a3281dda62de34cac5ce57c6d17ed7124933c6f..f65e06c84c989e4d00ac86b1344854415c60316d 100644 (file)
@@ -1,5 +1,5 @@
 
 
@@ -149,6 +149,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).
+   
+
  
 
  
index 41c6d1e2fb16314f8b8ed7d04dbce3e754dc0715..1fda61cc160f4215b9b7eede9c0f603e60cc1937 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/catalog/pg_proc.c,v 1.165 2009/09/22 23:43:37 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/catalog/pg_proc.c,v 1.166 2009/10/02 18:13:04 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -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);