OFF is a reserved keyword. It's not a reserved keyword in the SQL spec,
and it's not hard to see people using off as a variable or column name,
so it would be nice to relax that. To make things worse, OFFSET is also
a reserved keyword, which would be the other natural name for a variable
or column that stores an offset of some sort.

I bumped into this because we have a test case in the EDB regression
suite that uses 'off' as a PL/pgSQL variable name. It used to work
before 9.0, because PL/pgSQL variable names were replaced with $n-style
parameter markers before handing off the query to the backend parser.
It's a problem with all keywords in general, but 'off' seems like a
likely variable name in real applications, and there was no ambiguity
with it.

Looking at the grammar, OFF is only used here:
opt_boolean:
TRUE_P { $$ = "true"; }
FALSE_P { $$ = "false"; }
ON { $$ = "on"; }
OFF { $$ = "off"; }
;
And opt_boolean in turn is used in the following places:
var_value: opt_boolean
{ $$ = makeStringConst($1, @1); }
ColId_or_Sconst
{ $$ = makeStringConst($1, @1); }
NumericOnly
{ $$ = makeAConst($1, @1); }
;
...
copy_generic_opt_arg:
opt_boolean { $$ = (Node *) makeString($1); }
ColId_or_Sconst { $$ = (Node *) makeString($1); }
...
copy_generic_opt_arg_list_item:
opt_boolean { $$ = (Node *) makeString($1); }
ColId_or_Sconst { $$ = (Node *) makeString($1); }
;
...
explain_option_arg:
opt_boolean { $$ = (Node *) makeString($1); }
ColId_or_Sconst { $$ = (Node *) makeString($1); }
Note that ColId is also accepted alongside opt_boolean in all of those
with the same action, so if we just remove OFF from opt_boolean rule and
make it unreserved, nothing changes.

ECPG uses OFF as a keyword in its "SET autocommit = [ON | OFF]" rule, so
we have to retain it as an unreserved keyword, or make it an
ecpg-specific keyword in the ecpg grammar. But I don't know how to do
that, and it feels like a good idea to keep it in the unreserved keyword
list anyway, so I propose the attached patch.

Any objections? Any objections to backpatching to 9.0, where the
PL/pgSQL variable handling was changed?

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Search Discussions

  • Tom Lane at Oct 22, 2010 at 1:54 pm

    Heikki Linnakangas writes:
    OFF is a reserved keyword. It's not a reserved keyword in the SQL spec,
    and it's not hard to see people using off as a variable or column name,
    so it would be nice to relax that.
    While I can see the value of doing something about that, this seems
    awfully fragile:
    + /*
    + * OFF is also accepted as a boolean value, but is not listed
    + * here to avoid making it a reserved keyword. All uses of
    + * opt_boolean rule also accept a ColId with the same action -
    + * OFF is handled via that route.
    + */
    The production's correctness now depends on how it's used, and there's
    no way to prevent somebody from misusing it.

    I think it'd be better if you were to refactor the grammar so that ColId
    was actually one of the alternatives in this very production (call it
    opt_boolean_or_name, or something like that). Then at least there'd be
    less of a flavor of action-at-a-distance about the assumption that OFF
    was handled in a compatible fashion.

    regards, tom lane
  • Heikki Linnakangas at Oct 22, 2010 at 2:47 pm

    On 22.10.2010 16:54, Tom Lane wrote:
    Heikki Linnakangas<heikki.linnakangas@enterprisedb.com> writes:
    OFF is a reserved keyword. It's not a reserved keyword in the SQL spec,
    and it's not hard to see people using off as a variable or column name,
    so it would be nice to relax that.
    While I can see the value of doing something about that, this seems
    awfully fragile:
    + /*
    + * OFF is also accepted as a boolean value, but is not listed
    + * here to avoid making it a reserved keyword. All uses of
    + * opt_boolean rule also accept a ColId with the same action -
    + * OFF is handled via that route.
    + */
    The production's correctness now depends on how it's used, and there's
    no way to prevent somebody from misusing it.

    I think it'd be better if you were to refactor the grammar so that ColId
    was actually one of the alternatives in this very production (call it
    opt_boolean_or_name, or something like that). Then at least there'd be
    less of a flavor of action-at-a-distance about the assumption that OFF
    was handled in a compatible fashion.
    Ah yes, that's much better. Committed that way.

    --
    Heikki Linnakangas
    EnterpriseDB http://www.enterprisedb.com

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedOct 22, '10 at 7:43a
activeOct 22, '10 at 2:47p
posts3
users2
websitepostgresql.org...
irc#postgresql

2 users in discussion

Heikki Linnakangas: 2 posts Tom Lane: 1 post

People

Translate

site design / logo © 2021 Grokbase