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
