(2009/12/18 9:19), Tom Lane wrote:
> KaiGai Kohei<kaigai@ak.jp.nec.com> writes:
>> [ patch to remove EnableDisableRule's permissions check ]
>
> I don't particularly like this patch, mainly because I disagree with
> randomly removing permissions checks without any sort of plan about
> where they ought to go. There are two principal entry points in
> rewriteDefine.c (the other one being DefineQueryRewrite), and currently
> they both do permissions checks. There is also a third major function
> RenameRewriteRule, currently ifdef'd out because it's not used, which
> is commented as "Note that it lacks a permissions check", indicating
> that somebody (possibly me, I don't recall for sure) thought it was
> surprising that there wasn't such a check there. This is sensible if
> you suppose that this file implements rule utility commands that are
> more or less directly called by the user, which is in fact the case for
> DefineQueryRewrite, and it's not obvious why it wouldn't be the case for
> EnableDisableRule. Since that's a globally exposed function, it's quite
> possible that there's third-party code calling it and expecting it to do
> the appropriate permissions check. (A quick look at Slony, in
> particular, would be a good idea here.)
If we consider this permission check is a part of specification in
the EnableDisableRule(), it is a viewpoint/standpoint.
However, if we adopt this standpoint, we should skip ATSimplePermissions()
for ENABLE/DISABLE RULE options in ATPrepCmd(), because ATExecCmd() calls
EnableDisableRule() which applies permission checks according to the
specification.
We don't need to apply same checks twice. It will be enough with either
of them. But I don't think skipping ATSimplePermissions() is a right
direction, because the existing PG model obviously handles rewrite rules
as properties of relation, although it is not stored within pg_class.
So, it is quite natural to check permission to modify properties of
relations in ATPrepCmd().
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>