* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Right. In the "add to objname" cases, there is already some other > routine that initialized it previously by filling in some stuff; in the > case above, this happens in the getRelationIdentity() immediately > preceding this. > > In the other cases we initialize on that spot.
ahh, ok, that makes a bit more sense, sorry for missing it. Still makes
me wonder why objargs gets special treatment at the top of the function
and objnames doesn't- seems like both should be initialized either
before being passed in (and perhaps an Assert to verify that they are),
or they should both be initialized, but I tend to prefer just Assert'ing
that they are correct on entry- either both are valid pointers to empty
lists, or both NULL.
> > I'm also not a huge fan of the big object_type_map, but I also don't
> > have a better solution.
>
> Agreed. We have the ObjectProperty array also in the same file; it
> kinda looks like there is duplication here, but actually there isn't.
Yeah, I did notice that, and noticed that it's not duplication.
> This whole issue is just fallout from the fact that we have three
> different ways to identify object classes: the ObjectClass enum, the
> ObjectType enum, and the relation OIDs of each catalog (actually a
> fourth one, see below). I don't see any other nice way around this; I
> guess we could try to auto-generate these tables somehow from a master
> text file, or something. Not material for this patch, I think.
Agreed that this patch doesn't need to address it and not sure that a
master text file would actually be an improvement.. I had been thinking
if there was some way to have a single mapping which could be used in
either direction, but I didn't see any sensible way to make that work
given that it's not *quite* the same backwards and forewards.
> Note my DDL deparse patch adds a comment:
>
> +/* XXX merge this with ObjectTypeMap? */
> static event_trigger_support_data event_trigger_support[] = {
Yes, I saw that, and that you added a comment that the new map needs to
be updated when changes are made, which is also good.
> and a late revision to that patch added a new function in
> event_triggers.c (not yet posted I think) due to GRANT having its own
> enum of object types, AclObjectKind.
Yeah. Perhaps one day we'll unify all of these, though I'm not 100%
sure it'd be possible...
Thanks!
Stephen
signature.asc
Description: Digital signature
