Commit d691cb91 authored by Tom Lane's avatar Tom Lane

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.
parent caa4cfa3
<!-- <!--
$PostgreSQL: pgsql/doc/src/sgml/ref/create_function.sgml,v 1.86 2009/09/19 10:23:26 petere Exp $ $PostgreSQL: pgsql/doc/src/sgml/ref/create_function.sgml,v 1.87 2009/10/02 18:13:04 tgl Exp $
--> -->
<refentry id="SQL-CREATEFUNCTION"> <refentry id="SQL-CREATEFUNCTION">
...@@ -562,6 +562,14 @@ CREATE FUNCTION foo(int, int default 42) ... ...@@ -562,6 +562,14 @@ CREATE FUNCTION foo(int, int default 42) ...
<literal>USAGE</literal> privilege on the language. <literal>USAGE</literal> privilege on the language.
</para> </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>
<refsect1 id="sql-createfunction-examples"> <refsect1 id="sql-createfunction-examples">
......
<!-- <!--
$PostgreSQL: pgsql/doc/src/sgml/ref/create_view.sgml,v 1.41 2009/01/27 12:40:15 petere Exp $ $PostgreSQL: pgsql/doc/src/sgml/ref/create_view.sgml,v 1.42 2009/10/02 18:13:04 tgl Exp $
PostgreSQL documentation PostgreSQL documentation
--> -->
...@@ -149,6 +149,14 @@ CREATE VIEW vista AS SELECT text 'Hello World' AS hello; ...@@ -149,6 +149,14 @@ CREATE VIEW vista AS SELECT text 'Hello World' AS hello;
used by the view. used by the view.
</para> </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>
<refsect1> <refsect1>
......
...@@ -8,7 +8,7 @@ ...@@ -8,7 +8,7 @@
* *
* *
* IDENTIFICATION * 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, ...@@ -89,6 +89,7 @@ ProcedureCreate(const char *procedureName,
bool internalInParam = false; bool internalInParam = false;
bool internalOutParam = false; bool internalOutParam = false;
Oid variadicType = InvalidOid; Oid variadicType = InvalidOid;
Oid proowner = GetUserId();
Relation rel; Relation rel;
HeapTuple tup; HeapTuple tup;
HeapTuple oldtup; HeapTuple oldtup;
...@@ -290,7 +291,7 @@ ProcedureCreate(const char *procedureName, ...@@ -290,7 +291,7 @@ ProcedureCreate(const char *procedureName,
namestrcpy(&procname, procedureName); namestrcpy(&procname, procedureName);
values[Anum_pg_proc_proname - 1] = NameGetDatum(&procname); values[Anum_pg_proc_proname - 1] = NameGetDatum(&procname);
values[Anum_pg_proc_pronamespace - 1] = ObjectIdGetDatum(procNamespace); 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_prolang - 1] = ObjectIdGetDatum(languageObjectId);
values[Anum_pg_proc_procost - 1] = Float4GetDatum(procost); values[Anum_pg_proc_procost - 1] = Float4GetDatum(procost);
values[Anum_pg_proc_prorows - 1] = Float4GetDatum(prorows); values[Anum_pg_proc_prorows - 1] = Float4GetDatum(prorows);
...@@ -353,7 +354,7 @@ ProcedureCreate(const char *procedureName, ...@@ -353,7 +354,7 @@ ProcedureCreate(const char *procedureName,
(errcode(ERRCODE_DUPLICATE_FUNCTION), (errcode(ERRCODE_DUPLICATE_FUNCTION),
errmsg("function \"%s\" already exists with same argument types", errmsg("function \"%s\" already exists with same argument types",
procedureName))); procedureName)));
if (!pg_proc_ownercheck(HeapTupleGetOid(oldtup), GetUserId())) if (!pg_proc_ownercheck(HeapTupleGetOid(oldtup), proowner))
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_PROC, aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_PROC,
procedureName); procedureName);
...@@ -471,7 +472,10 @@ ProcedureCreate(const char *procedureName, ...@@ -471,7 +472,10 @@ ProcedureCreate(const char *procedureName,
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_proowner - 1] = false;
replaces[Anum_pg_proc_proacl - 1] = false; replaces[Anum_pg_proc_proacl - 1] = false;
...@@ -498,12 +502,11 @@ ProcedureCreate(const char *procedureName, ...@@ -498,12 +502,11 @@ ProcedureCreate(const char *procedureName,
/* /*
* Create dependencies for the new function. If we are updating an * Create dependencies for the new function. If we are updating an
* existing function, first delete any existing pg_depend entries. * 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) if (is_update)
{
deleteDependencyRecordsFor(ProcedureRelationId, retval); deleteDependencyRecordsFor(ProcedureRelationId, retval);
deleteSharedDependencyRecordsFor(ProcedureRelationId, retval, 0);
}
myself.classId = ProcedureRelationId; myself.classId = ProcedureRelationId;
myself.objectId = retval; myself.objectId = retval;
...@@ -537,7 +540,8 @@ ProcedureCreate(const char *procedureName, ...@@ -537,7 +540,8 @@ ProcedureCreate(const char *procedureName,
} }
/* dependency on owner */ /* dependency on owner */
recordDependencyOnOwner(ProcedureRelationId, retval, GetUserId()); if (!is_update)
recordDependencyOnOwner(ProcedureRelationId, retval, proowner);
heap_freetuple(tup); heap_freetuple(tup);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment