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

(2010/06/29 3:52), Robert Haas wrote:
> 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.
>
Sorry, I couldn't understand some of idiomatic expressions, but you are
saying that we have many variations of syscaches, so it is nonsense to
wrap up all the usecases of them, but being worthful to focus on major
usecases, such as name to oid translation, aren't you?

If so, it also seems to me fair enough. It seems to me translations from
name into oid, tuple or verifying object existence are major use cases,
but we have no reason why replace all the SysCacheSearch().

> 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.
>

--
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>