Re: [v9.3] OAT_POST_ALTER object access hooks

I rebased the patch towards the latest master branch, but here is no
functionality changes.

The part-1 patch adds catalog/objectaccess.c to have entrypoints of
object_access_hook, instead of simple macro definition, to simplify
invocations with arguments. It is just a replacement of existing
OAT_POST_CREATE and OAT_DROP invocation with new style,

The part-2 patch adds new OAT_POST_ALTER event type, and
its relevant permission checks on contrib/sepgsql.

I hope part-1 patch at least being committed earlier, because it performs
prerequisite of other two simple patches to add permission checks to
sepgsql, even though I can understand part-2 takes a bit more power
of concentration for reviewing.

Thanks,

2012/12/13 Kohei KaiGai <kaigai@kaigai.gr.jp>:
> 2012/12/11 Kohei KaiGai <kaigai@kaigai.gr.jp>:
>> 2012/12/11 Robert Haas <robertmhaas@gmail.com>:
>>> On Mon, Dec 3, 2012 at 9:59 AM, Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
>>>> As we discussed before, it is hard to determine which attributes shall
>>>> be informed to extension via object_access_hook, so the proposed
>>>> post-alter hook (that allows to compare older and newer versions)
>>>> works fine on 99% cases.
>>>> However, I'm inclined to handle SET TABLESPACE as an exception
>>>> of this scenario. For example, an idea is to define OAT_PREP_ALTER
>>>> event additionally, but only invoked very limited cases that takes
>>>> unignorable side-effects until system catalog updates.
>>>> For consistency of hook, OAT_POST_ALTER event may also ought
>>>> to be invoked just after catalog updates of pg_class->reltablespace,
>>>> but is_internal=true.
>>>>
>>>> How about your opinion?
>>>
>>> I don't really like it - I think POST should mean POST.  You can
>>> define some other hook that gets called somewhere else, but after
>>> means after.  There are other plausible uses of these hooks besides
>>> sepgsql; for example, logging the completion time of an action.
>>> Putting the hooks in the "wrong" places because that happens to be
>>> more convenient for sepgsql is going to render them useless for any
>>> other purpose.  Maybe nobody else will use them anyway, but I'd rather
>>> not render it impossible before we get off the ground.
>>>
>> I have to admit "PREP" hook is problematic to determine which information
>> should be delivered, and which should not, as we had discussed before.
>> Yes, a hook convenient for sepgsql, might be unconvenient for others.
>>
>> So, which alternatives do we have? I can list up a few ideas.
>>
>> 1) Putting POST_ALTER hook after existing whole table rewrite.
>> good: no need to have significant code change
>> bad: it takes heavy i/o prior to hook invocation
>>
>> 2) Updating reltablespace of pg_class prior to whole table rewrite,
>>     and put POST_ALTER hook just after catalog update.
>> good: well fit with assumption of POST hook.
>> bad: it takes significant code changes on table rewriting
>>
>> 3) Using ProcessUtility_hook to track AKTER TABLE ... SET
>>     TABLESPACE as an exception.
>> good: no need to have significant code change
>> bad: sepgsql also have to have analyzer of ALTER TABLE
>>         commands.
>>
>> I think (2) is the best way, if feasible. I need to investigate whether
>> the relevant code allows to implement catalog updates prior to
>> whole table rewriting.
>> However, (1) is also not so bad in a short term, as a first step towards
>> the idea (2) that takes additional code updates.
>> Of course, I can implement with (3), but not inclined with this idea.
>>
>> Is there other idea?
>>
> I tried to adjust my patch based on the idea (1).
>
> It does not do anything special on SET TABLESPACE, thus it follows
> the manner of "POST_ALTER" command, but it also means permission
> checks are applied after (possible) heavy i/o workload.
>
> I'd like to revise the logic of whole table rewrite to move catalog update
> prior to actual i/o works, however, it is not scope of this patch.
>
> Thanks,
> --
> KaiGai Kohei <kaigai@kaigai.gr.jp>



--
KaiGai Kohei <kaigai@kaigai.gr.jp>