Re: ExecutorCheckPerms() hook

(2010/05/25 12:19), Robert Haas wrote:
> On Mon, May 24, 2010 at 9:27 PM, Stephen Frost<sfrost(at)snowman(dot)net> wrote:
>> * KaiGai Kohei (kaigai(at)ak(dot)jp(dot)nec(dot)com) wrote:
>>> We have two options; If the checker function takes the list of RangeTblEntry,
>>> it will be comfortable to ExecCheckRTPerms(), but not DoCopy(). Inversely,
>>> if the checker function takes arguments in my patch, it will be comfortable
>>> to DoCopy(), but ExecCheckRTPerms().
>>>
>>> In my patch, it takes 6 arguments, but we can reference all of them from
>>> the given RangeTblEntry. On the other hand, if DoCopy() has to set up
>>> a pseudo RangeTblEntry to call checker function, it entirely needs to set
>>> up similar or a bit large number of variables.
>>
>> I don't know that it's really all that difficult to set up an RT in
>> DoCopy or RI_Initial_Check(). In my opinion, those are the strange or
>> corner cases- not the Executor code, through which all 'regular' DML is
>> done. It makes me wonder if COPY shouldn't have been implemented using
>> the Executor instead, but that's, again, a completely separate topic.
>> It wasn't, but it wants to play like it operates in the same kind of way
>> as INSERT, so it needs to pick up the slack.
>
> I think this approach is definitely worth investigating. KaiGai, can
> you please work up what the patch would look like if we do it this
> way?

OK, the attached patch reworks it according to the way.

* ExecCheckRTEPerms() becomes to take 2nd argument the caller to suggest
behavior on access violation. The 'abort' argument is true, it raises
an error using aclcheck_error() or ereport(). Otherwise, it returns
false immediately without rest of checks.

* DoCopy() and RI_Initial_Check() were reworked to call ExecCheckRTEPerms()
with locally built RangeTblEntry.

Thanks,
--
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>