From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Nikhils <nikkhils(at)gmail(dot)com> |
Cc: | "Dave Page" <dpage(at)pgadmin(dot)org>, "Peter Eisentraut" <peter_e(at)gmx(dot)net>, pgsql-patches(at)postgresql(dot)org, "Alvaro Herrera" <alvherre(at)commandprompt(dot)com> |
Subject: | Re: Exposing keywords to clients |
Date: | 2008-07-03 21:04:56 |
Message-ID: | 9155.1215119096@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-patches |
Nikhils <nikkhils(at)gmail(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Russell Smith | 2008-07-03 23:10:27 | Re: [PATCHES] Removal of the patches email list |
Previous Message | Garick Hamlin | 2008-07-03 18:55:33 | Re: Solaris ident authentication using unix domain sockets |