Thursday, July 3, 2008

Re: [PATCHES] Exposing keywords to clients

Nikhils <nikkhils@gmail.com> writes:
>>> Attached is an updated patch, giving the following output.

Applied with minor revisions.

> Here are some comments from me:

> a) Changed "localised" to "localized" to be consistent with the references
> elsewhere in the same file.

What I didn't like about the docs was the classification of the function
as a "session information function". There's certainly nothing
session-dependent about it. I ended up putting it under "System Catalog
Information Functions", which isn't really quite right either but it's
closer than the other one.

> b) I wonder if we need the default case in the switch statement at all,
> since we are scanning the statically populated ScanKeywords array with
> proper category values for each entry.

I left it in for safety, but made it just return nulls --- there's no
point in wasting more code space on it than necessary, and certainly
no point in asking translators to translate a useless string.

> c) There was a warning during compilation since we were assigning a const
> pointer to a char pointer
> values[0] = ScanKeywords[funcctx->call_cntr].name;

Yeah, I couldn't see a good way around that either --- we could pstrdup
but that seems overkill, and adjusting the API of BuildTupleFromCStrings
to take const char ** doesn't look appetizing either.

> d) oid 2700 has been claimed by another function in the meanwhile. Modified
> it to 2701.
> DATA(insert OID = 2701 ( pg_get_keywords PGNSP PGUID 12 10 400 f f t t s
> 0 2249

2701 was claimed too. You need to use the unused_oids script to find
unique free OIDs.

regards, tom lane

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

No comments: