2011/11/21 Robert Haas <robertmhaas@gmail.com>:
>>> Now, what you have here is a much broader reworking. And that's not
>>> necessarily bad, but at the moment I'm not really seeing how it
>>> benefits us.
>>>
>> In my point, if individual object types need to have its own handler for
>> alter commands, points of the code to check permissions are also
>> distributed for each object types. It shall be a burden to maintain hooks
>> that allows modules (e.g sepgsql) to have permission checks.
>
> Well, it's always nicer if you can just put a call to some hook in one
> place instead of many. But it's not worth sacrificing everything to
> make that happen. I think we need to compare the value of only
> needing a hook in one place against the disruption of changing a lot
> of code that is working fine as it is. In the case of the DROP
> commands, it seems to me that the refactoring you did came out a huge
> win, but this doesn't seem as clear to me. Note that DROP actually
> does dispatch the actual work of dropping the object to a
> type-specific function, unlike what you're trying to do here.
>
Yes, I agree with your opinion.
I'm also not sure whether my refactoring efforts on ALTER commands
will give us larger worth than its size of code changes, although we will
be able to consolidate the point of hooks around them.
If we add new properties that required by AlterObjectNamespace, as
you suggested, it will allow to reduce number of caller of this routine
mechanically with small changes.
Should I try this reworking with this way?
At least, AlterObjectNamespace already consolidate the point to check
permissions.
I initially thought it is too specific for AlterObjectNamespace, then I
reconsidered that we may be able to apply same properties to
ALTER RENAME TO/SET OWNER commands also, even though
these commands have completely branched routines for each object
types.
Thanks,
--
KaiGai Kohei <kaigai@kaigai.gr.jp>
