Re: [v9.3] OAT_POST_ALTER object access hooks

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>