On Mon, Jun 28, 2010 at 10:53 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> 2010/6/28 KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>:
>>> * at the RenameSchema()
>> This looks like another syscache reference leak.
> Actually that one *is* a leak, although it doesn't matter much because
> the leak occurs only in an error path, so transaction abort will clean
> up the leaked reference. Still, it's sucky coding style:
> SearchSysCacheExists should have been used.
> I'm not sure I agree that replacing SearchSysCacheExists calls (or
> things that should have been SearchSysCacheExists calls) with
> OidIsValid(get_whatever_oid()) is an improvement. The Exists call
> tells what you're actually trying to accomplish. The other way is
> an overspecification of the required result.
It is, but on the other hand it's only good fortune that testing
whether a schema exists is a one-liner even without using
get_namespace_oid(). Take a look at get_tablespace_oid() in CVS HEAD
[which BTW is already used in this style], or at get_trigger_oid() in
the "get_whatever_oid, part 2" patch, for example. I think the
assumption that system objects (with the exception of attributes) are
identified by OIDs is embedded pretty deeply in the code, so I don't
think it costs us much to rely on it. We're more likely to want to do
things like add or remove a syscache, in which case a style that
minimizes direct syscache references is apt to make life easier.
It's also slightly less wordy, which I like, too.
The Enterprise Postgres Company
In response to
pgsql-hackers by date
|Next:||From: Robert Haas||Date: 2010-06-28 16:07:16|
|Subject: Re: Issue: Deprecation of the XML2 module 'xml_is_well_formed' function|
|Previous:||From: David Fetter||Date: 2010-06-28 16:00:47|
|Subject: Re: Issue: Deprecation of the XML2 module