SEO

#1 (**permalink**) 05-05-2008, 05:53 AM

Dave Page wrote:
> Hi,
>
> The attached patch implements a new function, pg_get_keywords(), which
> returns a set of records describing the keywords recognised by the
> server. This allows clients such as pgAdmin to get quoting rules
> correct, and helps with other tasks such as syntax highlighting where
> we need to support multiple server versions.

FWIW pg_dump has fmtId() which does something related.

I think it's a bit bogus to be using the list as compiled client-side,
precisely due to the theoretical chance that it could change from one
server version to the next, but it's probably not very likely that we
ever remove a keyword from the server grammar. And highlighting a
keyword that's not really a keyword is unlikely to be a problem in
practice -- in fact it makes it obvious that the user is likely to be in
trouble later when they upgrade.

> postgres=# select * from pg_get_keywords();
> word | category
> -------------------+-----------------------
> all | Reserved
> binary | Type or function name
> xmlserialize | Column name
> zone | Unreserved
> (372 rows)
>
> I wasn't sure about the best way to describe the categories -
> obviously they need to be non-translatable (for client software to
> interpret), but human readable is also nice. I'm happy to hear
> alternate suggestions.

Perhaps use a separate string for machine parse (say R, T, C, U), and
let the string be translatable.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

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

#2 (**permalink**) 05-05-2008, 05:53 AM

Alvaro Herrera <alvherre@commandprompt.com> writes:
> FWIW pg_dump has fmtId() which does something related.

> I think it's a bit bogus to be using the list as compiled client-side,
> precisely due to the theoretical chance that it could change from one
> server version to the next, but it's probably not very likely that we
> ever remove a keyword from the server grammar.

Actually, it's 100% intentional that pg_dump does it that way --- I
would not support modifying it to use this function (even if it existed
in the back branches). The reason is exactly that pg_dump wants to
generate output that is correct for its own PG version, not that of the
server it's dumping from.

The tradeoffs are probably different for pgAdmin, but it is important to
realize that either way might be the best thing for a particular case.

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

#3 (**permalink**) 05-05-2008, 05:53 AM

On Sat, May 3, 2008 at 12:24 AM, Alvaro Herrera
<alvherre@commandprompt.com> wrote:
> Dave Page wrote:
> > Hi,
> >
> > The attached patch implements a new function, pg_get_keywords(), which
> > returns a set of records describing the keywords recognised by the
> > server. This allows clients such as pgAdmin to get quoting rules
> > correct, and helps with other tasks such as syntax highlighting where
> > we need to support multiple server versions.
>
> FWIW pg_dump has fmtId() which does something related.
>
> I think it's a bit bogus to be using the list as compiled client-side,
> precisely due to the theoretical chance that it could change from one
> server version to the next, but it's probably not very likely that we
> ever remove a keyword from the server grammar. And highlighting a
> keyword that's not really a keyword is unlikely to be a problem in
> practice -- in fact it makes it obvious that the user is likely to be in
> trouble later when they upgrade.

Yeah, we currently lift a copy of keywords.c into the pgAdmin source
and use that in our own qtIdent() function, but it's clearly only
correct for the version of Postgres we pull it from, whilst pgAdmin
supports back to 7.3 in current releases. It's also a pain because we
try to support some of the derivative servers like those offered by
Greenplum and EnterpriseDB which may have additional keywords (though
that obviously is not a problem for this community).

> > postgres=# select * from pg_get_keywords();
> > word | category
> > -------------------+-----------------------
> > all | Reserved
> > binary | Type or function name
> > xmlserialize | Column name
> > zone | Unreserved
> > (372 rows)
> >
> > I wasn't sure about the best way to describe the categories -
> > obviously they need to be non-translatable (for client software to
> > interpret), but human readable is also nice. I'm happy to hear
> > alternate suggestions.
>
> Perhaps use a separate string for machine parse (say R, T, C, U), and
> let the string be translatable.

I considered that, but wasn't sure if folks would like the redundancy.
It's trivial to do of course - any objections?

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

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

#4 (**permalink**) 05-05-2008, 05:53 AM

Dave Page wrote:
> > Perhaps use a separate string for machine parse (say R, T, C, U), and
> > let the string be translatable.
>
> I considered that, but wasn't sure if folks would like the redundancy.
> It's trivial to do of course - any objections?

Is there anything useful you would do with this information? Or would you
just quote all listed words anyway?

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

#5 (**permalink**) 05-05-2008, 05:53 AM

Peter Eisentraut <peter_e@gmx.net> writes:
> Dave Page wrote:
>>> Perhaps use a separate string for machine parse (say R, T, C, U), and
>>> let the string be translatable.
>>
>> I considered that, but wasn't sure if folks would like the redundancy.
>> It's trivial to do of course - any objections?

> Is there anything useful you would do with this information? Or would you
> just quote all listed words anyway?

I think the practical application would be to avoid quoting unreserved
keywords, as pg_dump for instance already does. I doubt anyone would
bother distinguishing the different types of partially/wholly reserved
words. So maybe a boolean would be sufficient --- but I have nothing
against the R/T/C/U suggestion.

A more radical alternative is just to omit unreserved words from the
view altogether.

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

#6 (**permalink**) 05-05-2008, 05:53 AM

"Dave Page" <dpage@pgadmin.org> writes:
> Attached is an updated patch, giving the following output. The catdesc
> column can be translated.

Documentation has got a couple of problems:

> + contains the keyword, the <structfield>catcode</> column contains a
> + category code of 'U' for unknown, 'C' for column name, 'T' for type or
> + function name or 'U' for unreserved. The <structfield>catdesc</>
> + column contains a localised stribg describing the category.

I wouldn't document the "unknown" case at all, much less document it
incorrectly, and you forgot the "reserved" case, and "stribg"?

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

#7 (**permalink**) 05-05-2008, 05:53 AM

"Dave Page" <dpage@pgadmin.org> writes:
> Attached is an updated patch, giving the following output.

Oh, one other thing: dropping externs into random modules unrelated to
their source module is completely awful programming style, because there
is nothing preventing incompatible declarations. Put those externs in
keywords.h instead. I suspect you have ignored a compiler warning
about not declaring pg_get_keywords itself, too --- it should be
extern'd in builtins.h.

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

Thread Tools
Show Printable Version Email this Page
Display Modes
Linear Mode Switch to Hybrid Mode Switch to Threaded Mode

SEO

Re: Exposing keywords to clients