Re: ExecutorCheckPerms() hook

(2010/05/25 12:19), Robert Haas wrote:
> On Mon, May 24, 2010 at 9:27 PM, Stephen Frost<sfrost@snowman.net>  wrote:
>> * KaiGai Kohei (kaigai@ak.jp.nec.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@ak.jp.nec.com>