Re: Refactoring on DROP/ALTER SET SCHEMA/ALTER RENAME TO statement

On Tue, Nov 15, 2011 at 4:43 AM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
> Part-2) Groundworks on objectaddress.c
> This patch adds necessary groundworks for Part-3 and Part-4.
> It adds ObjectPropertyType of objectaddress.c index-oid and cache-id
> for name lookup and attribute number of object name; these field is
> used to detect namespace conflict on object_exists_namespace() that
> shall be called on refactored ALTER SET SCHEMA/RENAME TO.
> It also reduce some arguments of check_object_ownership(), and allows
> to call this function without knowledge of ObjectType and text
> representation. It allows to check ownership using this function from
> the code path of AlterObjectNamespace_oid() that does not have (or
> need to look up catalog to obtain) ObjectType information.

I spent some time wading through the code for parts 2 through 4, and I
guess I'm not sure this is headed in the right direction. If we need
this much infrastructure in order to consolidate the handling of ALTER
BLAH .. SET SCHEMA, then maybe it's not worth doing.

But I'm not sure why we do. My thought here was that we should
extended the ObjectProperty array in objectaddress.c so that
AlterObjectNamespace can get by with fewer arguments - specifically,
it seems like the following ought to be things that can be looked up
via the ObjectProperty mechanism:

int oidCacheId, int nameCacheId, int Anum_name, int Anum_namespace,
int Anum_owner, AclObjectKind acl_kind

The advantage of that, or so it seems to me, is that all this
information is in a table in objectaddress.c where multiple clients
can get at it at need, as opposed to the current system where it's
passed as arguments to AlterObjectNamespace(), and all that would need
to be duplicated if we had another function that did something
similar.

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.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company