Re: Exposing keywords to clients

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

In response to

Browse pgsql-patches by date

  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