From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Stephen Frost <sfrost(at)snowman(dot)net>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: [PATCH] remove redundant ownership checks |
Date: | 2009-12-22 16:20:42 |
Message-ID: | 603c8f070912220820x35279883ua51affcb513c087b@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
2009/12/22 KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>:
> (2009/12/21 12:53), Robert Haas wrote:
>> On Thu, Dec 17, 2009 at 7:19 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> KaiGai Kohei<kaigai(at)ak(dot)jp(dot)nec(dot)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.
>>
>> So where ought they to go?
>>
>>> If we're going to start moving these checks around we need a very
>>> well-defined notion of where permissions checks should be made, so that
>>> everyone knows what to expect. I have not seen any plan for that.
>>
>> This seems to me to get right the heart of the matter. When I
>> submitted my machine-readable explain patch, you critiqued it for
>> implementing half of an abstraction layer, and it seems to me that our
>> current permissions-checking logic has precisely the same issue. We
>> consistently write code that starts by checking permissions and then
>> moves right along to implementing the action. Those two things need
>> to be severed. I see two ways to do this. Given a function that (A)
>> does some prep work, (B) checks permissions, and (C) performs the
>> action, we could either:
>>
>> 1. Make the existing function do (A) and (B) and then call another
>> function to do (C), or
>> 2. Make the existing function do (A), call another function to do (B),
>> and then do (C) itself.
>>
>> I'm not sure which will work better, but I think making a decision
>> about which way to do it and how to name the functions would be a big
>> step towards having a coherent plan for this project.
>
> We have mixed policy in the current implementation.
>
> The point is what database object shall be handled in this function.
>
> If we consider a rewrite rule as a database object, not a property of
> the relation, it seems to me a correct manner to apply permission checks
> in the EnableDisableRule(), because it handles a given rewrite rule.
>
> If we consider a rewrite rule as a property of a relation, not an independent
> database object, it seems to me we should apply permission checks in ATPrepCmd()
> which handles a relation, rather than EnableDisableRule().
>
> My patch stands on the later perspective, because pg_rewrite entries don't
> have its own ownership and access privileges, and it is always owned by
> a certain relation.
That's somewhat separate from the point I was making, but it's a good
point all the same.
...Robert
From | Date | Subject | |
---|---|---|---|
Next Message | Simon Riggs | 2009-12-22 16:25:37 | Re: alpha3 release schedule? |
Previous Message | Heikki Linnakangas | 2009-12-22 16:17:42 | Re: alpha3 release schedule? |