[PATCH] Remove obscure permission checks in FindConversion()

This patch fixes a problem discussed earlier.

Now, FindConversion() which is only called from FindConversionByName()
checks ACL_EXECUTE permission on the conversion procedure matched.
If not allowed, it performs as if the given conversion does not exist
(InvalidOid shall be returned).

The FindConversionByName() is called from:
 - CommentConversion()
 - DropConversionsCommand()
 - RenameConversion()
 - AlterConversionOwner()

All of them also check ownership of the conversion eventually.
In addition, CreateConversionCommand() checks ACL_EXECUTE permission
on the conversion procedure when creation time, independently.

Also see the related discussion:
  http://archives.postgresql.org/pgsql-hackers/2009-08/msg01361.php
  http://archives.postgresql.org/pgsql-hackers/2009-08/msg01392.php
  http://archives.postgresql.org/pgsql-hackers/2009-08/msg01420.php

Thanks,

(2009/08/20 8:50), KaiGai Kohei wrote:
> Tom Lane wrote:
>> KaiGai Kohei<kaigai@ak.jp.nec.com>  writes:
>>> When FindConversion() is called, it also checks current user's ACL_EXECUTE
>>> privilege on the conproc of the fetched conversion.
>>
>>> Why this check is applied on FindConversion(), instead of FindDefaultConversion()?
>>
>> Does seem pretty inconsistent, doesn't it?
>
> Anyway, I could not understand the intention of the checks, rather
> than inconsistency. So, I doubted it might be a bug, if it intended
> to provide a permission something like ACL_USAGE on conversion.
>
>> The original idea may have been to provide a substitute for a USAGE
>> ACL check on conversions, in which case it's not totally insane: if
>> you make a conversion default then you're implicitly granting it to
>> public.  But there's no documentation about this.
>
> If ACL_EXECUTE checks is deployed on FindDefaultConversion(), it performs
> as if something like ACL_USAGE permission. However, FindConversion() is
> called from ALTER or DROP CONVERSION, so it controls DDL statements
> in addition to its ownership.
> Please note that this check (ACL_EXECUTE) is not applied when user choose
> a certain conversion.
>
>> Offhand I see no really good reason to have a usage check on
>> conversions, and would be happy with removing this one.  The function
>> permission check at CREATE CONVERSION time ought to be sufficient.
>
> It seems to me reasonable.
>
> If user does not have ACL_EXECUTE privilege on the function used in
> the conversion, he does not alter or drop the conversion, even if he
> has ownership of the conversion. This behavior is hard to understand.
>
> Thanks,


--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>