Re: [PATCH] remove redundant ownership checks

(2010/01/14 4:54), Tom Lane wrote:
> Robert Haas<robertmhaas(at)gmail(dot)com> writes:
>> On Wed, Jan 13, 2010 at 1:34 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> If I thought this patch represented incremental movement in the
>>> direction of a better security-check factorization, I'd be fine with it,
>>> but that's not clear either. �The argument for it is that these checks
>>> are redundant with some other ones, but why should we remove these and
>>> not the other ones instead?
>
>> That's a good question, and I have an answer [ namely that ALTER TABLE
>> is the right place ].
>
> But note Stephen Frost's concurrent reply suggesting that he wants to
> move the checks *out* of ALTER TABLE. With his plan, these checks
> are probably in the right place already.

Note that this patch tries to remove redundant checks in this code path.
If ATPrepCmd() would not be a right place to apply permission checks,
we should remove invocation of the ATSimplePermissions() for AT_EnableRule
and so on. (Of course, we need to copy two other sanity check in the
ATSimplePermissions() also)

However, in my opinion, ATPrepCmd() is more appropriate to apply permission
checks than EnableDisableRule(), because we deal with rewrite rule (that
does not have individual ownership and acls) as properties of a relation,
not an independent database object, although it is stored in its own
system catalog. It is quite natural to check privileges to alter properties
of a relaion in tablecmd.c, rather than rewriteDefine.c.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>