Re: get_whatever_oid, part 1: object types with unqualifed names

On Mon, Jun 28, 2010 at 1:00 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Another, and related idea that I had while looking at this is that a
>> lot of object types could benefit from a get_whatever_heaptuple()
>> function with the same calling syntax.  get_whatever_oid() could be
>> restructured to use it, and most object types would have other
>> callers, also.  But that too seems like opening a larger can of worms
>> than I really want to get into at this point.
>
> This is the sort of thing that I think we should get right the first
> time, rather than have multiple waves of large-scale changes.

After taking a walk around the block, I have two further thoughts about this:

1. I wouldn't have submitted these patches (this one and the part two
patch) unless it reflected my best judgment about how far it makes
sense to proceed with refactoring in a certain direction. I'm willing
to be second-guessed, if you or someone else wants to move the
goal-posts. But I don't personally feel that there's enough bang for
the buck in going further in this direction, or I would have done it
already. I'm not planning to commit these patches and then start
immediately working on a second set of patches that touches all of
these same spots in the code over again. In the particular area that
this patch touches (mapping names as gathered by the parser to OIDs),
there are many different permutations: the input can be a cstring, or
a text datum, or a list of cstrings, etc.; and the output can be a
boolean (is it there?), an OID, a heap tuple, etc. If you try to
cover every combination, especially for the more obscure object types,
you'll drive yourself nuts; on the other hand, trying to regularize
the more common cases is, I think, helpful and worthwhile. So it's a
trade-off; I took my best crack at it.

2. It might be too optimistic to think that we're going to avoid
having large-scale code changes in 9.1 by committing these to 9.0. I
think refactoring is a fact of life as we try to move the project
forward, and while we want to be careful about how we do it for the
reasons you mention, it's also important if we want to have a clean
base to build on for future features (which, in fact, is why I
proposed these patches in the first place - I discovered that this
code wasn't too clean right now while thinking about SE-PostgreSQL
security labels at PGCon). I have at least one other patch that's
basically just refactoring in the queue for 9.1 already, which is
fairly wide-ranging but in an area completely unrelated to these
patches, and a couple other less ambitious ones that I plan to work on
as time permits; so I think that the need for these kinds of changes
is not going to go away.

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