Обсуждение: SQL/JSON functions vs. ECPG vs. STRING as a reserved word

Поиск
Список
Период
Сортировка

SQL/JSON functions vs. ECPG vs. STRING as a reserved word

От
Tom Lane
Дата:
Commit 1a36bc9db (SQL/JSON query functions) introduced STRING as a
type_func_name_keyword.  As per the complaint at [1], this broke use
of "string" as a table name, column name, function parameter name, etc.
The restriction about column names, in particular, seems likely to
break boatloads of applications --- wouldn't you think that "string"
is a pretty likely column name?  We have no cover to claim "the SQL
standard says so", either, because they list STRING as an unreserved
keyword.

This is trivial enough to fix so far as the core grammar is concerned;
it works to just change STRING to be an unreserved_keyword.  However,
various ECPG tests fall over, so I surmise that somebody felt it was
okay to break potentially thousands of applications in order to avoid
touching ECPG.  I do not think that's an acceptable trade-off.

I poked into it a bit and found the proximate cause: ECPG uses
ECPGColLabelCommon to represent user-chosen type names in some
places, and that production *does not include unreserved_keyword*.
(Sure enough, the failing test cases try to use "string" as a
type name in a variable declaration.)  That's a pre-existing
bit of awfulness, and it's indeed pretty awful, because it means
that any time we add a new keyword --- even a fully unreserved one
--- we risk breaking somebody's ECPG application.  We just hadn't
happened to add any keywords to date that conflicted with type names
used in the ECPG test suite.

I looked briefly at whether we could improve that situation.
Two of the four uses of ECPGColLabelCommon in ecpg.trailer can be
converted to the more general ECPGColLabel without difficulty,
but trying to change either of the uses in var_type results in
literally thousands of shift/reduce and reduce/reduce conflicts.
This seems to be because what follows ecpgstart can be either a general
SQL statement or an ECPGVarDeclaration (beginning with var_type),
and bison isn't smart enough to disambiguate.  I have a feeling that
this situation could be improved with enough elbow grease, because
plpgsql manages to solve a closely-related problem in allowing its
assignment statements to coexist with general SQL statements.  But
I don't have the interest to tackle it personally, and anyway any
fix would probably be more invasive than we want to put in post-beta.

I also wondered briefly about just changing the affected test cases,
reasoning that breaking some ECPG applications that use "string" is
less bad than breaking everybody else that uses "string".  But type
"string" is apparently a standard ECPG and/or INFORMIX thing, so we
probably can't get away with that.

Hence, I propose the attached, which fixes the easily-fixable
ECPGColLabelCommon uses and adds a hard-wired special case for
STRING to handle the var_type usage.

More generally, I feel like we have a process problem: there needs to
be a higher bar to adding new fully- or even partially-reserved words.
I might've missed it, but I don't recall that there was any discussion
of the compatibility implications of this change.

            regards, tom lane

[1]
https://www.postgresql.org/message-id/PAXPR02MB760049C92DFC2D8B5E8B8F5AE3DA9%40PAXPR02MB7600.eurprd02.prod.outlook.com

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 989db0dbec..969c9c158f 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -17940,6 +17940,7 @@ unreserved_keyword:
             | STORAGE
             | STORED
             | STRICT_P
+            | STRING
             | STRIP_P
             | SUBSCRIPTION
             | SUPPORT
@@ -18098,7 +18099,6 @@ type_func_name_keyword:
             | OVERLAPS
             | RIGHT
             | SIMILAR
-            | STRING
             | TABLESAMPLE
             | VERBOSE
         ;
diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h
index 8a2ab405a2..ae35f03251 100644
--- a/src/include/parser/kwlist.h
+++ b/src/include/parser/kwlist.h
@@ -426,7 +426,7 @@ PG_KEYWORD("stdout", STDOUT, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("storage", STORAGE, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("stored", STORED, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("strict", STRICT_P, UNRESERVED_KEYWORD, BARE_LABEL)
-PG_KEYWORD("string", STRING, TYPE_FUNC_NAME_KEYWORD, BARE_LABEL)
+PG_KEYWORD("string", STRING, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("strip", STRIP_P, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("subscription", SUBSCRIPTION, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("substring", SUBSTRING, COL_NAME_KEYWORD, BARE_LABEL)
diff --git a/src/interfaces/ecpg/preproc/ecpg.trailer b/src/interfaces/ecpg/preproc/ecpg.trailer
index daf979a8e8..b95fc44314 100644
--- a/src/interfaces/ecpg/preproc/ecpg.trailer
+++ b/src/interfaces/ecpg/preproc/ecpg.trailer
@@ -467,7 +467,7 @@ type_declaration: S_TYPEDEF
         /* an initializer specified */
         initializer = 0;
     }
-    var_type opt_pointer ECPGColLabelCommon opt_array_bounds ';'
+    var_type opt_pointer ECPGColLabel opt_array_bounds ';'
     {
         add_typedef($5, $6.index1, $6.index2, $3.type_enum, $3.type_dimension, $3.type_index, initializer, *$4 ? 1 :
0);

@@ -701,6 +701,43 @@ var_type:    simple_type
                 struct_member_list[struct_level] = ECPGstruct_member_dup(this->struct_member_list);
             }
         }
+        | STRING
+        {
+            /*
+             * It's quite horrid that ECPGColLabelCommon excludes
+             * unreserved_keyword, meaning that unreserved keywords can't be
+             * used as type names in var_type.  However, this is hard to avoid
+             * since what follows ecpgstart can be either a random SQL
+             * statement or an ECPGVarDeclaration (beginning with var_type).
+             * Pending a bright idea about how to fix that, we must
+             * special-case STRING (and any other unreserved keywords that are
+             * likely to be needed here).
+             */
+            if (INFORMIX_MODE)
+            {
+                $$.type_enum = ECPGt_string;
+                $$.type_str = mm_strdup("char");
+                $$.type_dimension = mm_strdup("-1");
+                $$.type_index = mm_strdup("-1");
+                $$.type_sizeof = NULL;
+            }
+            else
+            {
+                /* this is for typedef'ed types */
+                struct typedefs *this = get_typedef("string");
+
+                $$.type_str = (this->type->type_enum == ECPGt_varchar || this->type->type_enum == ECPGt_bytea) ? EMPTY
:mm_strdup(this->name); 
+                $$.type_enum = this->type->type_enum;
+                $$.type_dimension = this->type->type_dimension;
+                $$.type_index = this->type->type_index;
+                if (this->type->type_sizeof && strlen(this->type->type_sizeof) != 0)
+                    $$.type_sizeof = this->type->type_sizeof;
+                else
+                    $$.type_sizeof = cat_str(3, mm_strdup("sizeof("), mm_strdup(this->name), mm_strdup(")"));
+
+                struct_member_list[struct_level] = ECPGstruct_member_dup(this->struct_member_list);
+            }
+        }
         | s_struct_union_symbol
         {
             /* this is for named structs/unions */
@@ -1342,7 +1379,7 @@ ECPGTypedef: TYPE_P
             /* an initializer specified */
             initializer = 0;
         }
-        ECPGColLabelCommon IS var_type opt_array_bounds opt_reference
+        ECPGColLabel IS var_type opt_array_bounds opt_reference
         {
             add_typedef($3, $6.index1, $6.index2, $5.type_enum, $5.type_dimension, $5.type_index, initializer, *$7 ? 1
:0); 


Re: SQL/JSON functions vs. ECPG vs. STRING as a reserved word

От
Michael Meskes
Дата:
> I looked briefly at whether we could improve that situation.
> Two of the four uses of ECPGColLabelCommon in ecpg.trailer can be
> converted to the more general ECPGColLabel without difficulty,
> but trying to change either of the uses in var_type results in
> literally thousands of shift/reduce and reduce/reduce conflicts.
> This seems to be because what follows ecpgstart can be either a general
> SQL statement or an ECPGVarDeclaration (beginning with var_type),
> and bison isn't smart enough to disambiguate.  I have a feeling that
> this situation could be improved with enough elbow grease, because
> plpgsql manages to solve a closely-related problem in allowing its
> assignment statements to coexist with general SQL statements.  But
> I don't have the interest to tackle it personally, and anyway any
> fix would probably be more invasive than we want to put in post-beta.

Right, the reason for all this is that the part after the "exec sql" could be
either language, SQL or C. It has been like this for all those years. I do not
claim that the solution we have is the best, it's only the best I could come up
with when I implemented it. If anyone has a better way, please be my guest.

> I also wondered briefly about just changing the affected test cases,
> reasoning that breaking some ECPG applications that use "string" is
> less bad than breaking everybody else that uses "string".  But type
> "string" is apparently a standard ECPG and/or INFORMIX thing, so we
> probably can't get away with that.

IIRC STRING is a normal datatype for INFORMIX and thus is implemented in ECPG
for its compatibility.

> Hence, I propose the attached, which fixes the easily-fixable
> ECPGColLabelCommon uses and adds a hard-wired special case for
> STRING to handle the var_type usage.

I'm fine with this approach.

Thanks Tom.

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org



Re: SQL/JSON functions vs. ECPG vs. STRING as a reserved word

От
Tom Lane
Дата:
Michael Meskes <meskes@postgresql.org> writes:
>> This seems to be because what follows ecpgstart can be either a general
>> SQL statement or an ECPGVarDeclaration (beginning with var_type),
>> and bison isn't smart enough to disambiguate.  I have a feeling that
>> this situation could be improved with enough elbow grease, because
>> plpgsql manages to solve a closely-related problem in allowing its
>> assignment statements to coexist with general SQL statements.

> Right, the reason for all this is that the part after the "exec sql" could be
> either language, SQL or C. It has been like this for all those years. I do not
> claim that the solution we have is the best, it's only the best I could come up
> with when I implemented it. If anyone has a better way, please be my guest.

I pushed the proposed patch, but after continuing to think about
it I have an idea for a possible solution to the older problem.
I noticed that the problematic cases in var_type aren't really
interested in seeing any possible unreserved keyword: they care about
certain specific built-in type names (most of which are keywords
already) and about typedef names.  Now, almost every C-parsing program
I've ever seen has to lex typedef names specially, so what if we made
ecpg do that too?  After a couple of false starts, I came up with the
attached draft patch.  The key ideas are:

1. In pgc.l, if an identifier is a typedef name, ignore any possible
keyword meaning and return it as an IDENT.  (I'd originally supposed
that we'd want to return some new TYPEDEF token type, but that does
not seem to be necessary right now, and adding a new token type would
increase the patch footprint quite a bit.)

2. In the var_type production, forget about ECPGColLabel[Common]
and just handle the keywords we know we need, plus IDENT for the
typedef case.  It turns out that we have to have duplicate coding
because most of these keywords are not keywords in C lexing mode,
so that they'll come through the IDENT path anyway when we're
in a C rather than SQL context.  That seemed acceptable to me.
I thought about adding them all to the C keywords list but that
seemed likely to have undesirable side-effects, and again it'd
bloat the patch footprint.

This fix is not without downsides.  Disabling recognition of
keywords that match typedefs means that, for example, if you
declare a typedef named "work" then ECPG will fail to parse
"EXEC SQL BEGIN WORK".  So in a real sense this is just trading
one hazard for another.  But there is an important difference:
with this, whether your ECPG program works depends only on what
typedef names and SQL commands are used in the program.  If
it compiles today it'll still compile next year, whereas with
the present implementation the addition of some new unreserved
SQL keyword could break it.  We'd have to document this change
for sure, and it wouldn't be something to back-patch, but it
seems like it might be acceptable from the users' standpoint.

We could narrow (not eliminate) this hazard if we could get the
typedef lookup in pgc.l to happen only when we're about to parse
a var_type construct.  But because of Bison's lookahead behavior,
that seems to be impossible, or at least undesirably messy
and fragile.  But perhaps somebody else will see a way.

Anyway, this seems like too big a change to consider for v15,
so I'll stick this patch into the v16 CF queue.  It's only
draft quality anyway --- lacks documentation changes and test
cases.  There are also some coding points that could use review.
Notably, I made the typedef lookup override SQL keywords but
not C keywords; this is for consistency with the C-mode lookup
rules, but is it the right thing?

            regards, tom lane

diff --git a/src/interfaces/ecpg/preproc/ecpg.trailer b/src/interfaces/ecpg/preproc/ecpg.trailer
index b95fc44314..fba35f6be6 100644
--- a/src/interfaces/ecpg/preproc/ecpg.trailer
+++ b/src/interfaces/ecpg/preproc/ecpg.trailer
@@ -564,8 +564,29 @@ var_type:    simple_type
             $$.type_index = mm_strdup("-1");
             $$.type_sizeof = NULL;
         }
-        | ECPGColLabelCommon '(' precision opt_scale ')'
+        | NUMERIC '(' precision opt_scale ')'
         {
+            $$.type_enum = ECPGt_numeric;
+            $$.type_str = mm_strdup("numeric");
+            $$.type_dimension = mm_strdup("-1");
+            $$.type_index = mm_strdup("-1");
+            $$.type_sizeof = NULL;
+        }
+        | DECIMAL_P '(' precision opt_scale ')'
+        {
+            $$.type_enum = ECPGt_decimal;
+            $$.type_str = mm_strdup("decimal");
+            $$.type_dimension = mm_strdup("-1");
+            $$.type_index = mm_strdup("-1");
+            $$.type_sizeof = NULL;
+        }
+        | IDENT '(' precision opt_scale ')'
+        {
+            /*
+             * In C parsing mode, NUMERIC and DECIMAL are not keywords, so
+             * they will show up here as a plain identifier, and we need
+             * this duplicate code to recognize them.
+             */
             if (strcmp($1, "numeric") == 0)
             {
                 $$.type_enum = ECPGt_numeric;
@@ -587,15 +608,98 @@ var_type:    simple_type
             $$.type_index = mm_strdup("-1");
             $$.type_sizeof = NULL;
         }
-        | ECPGColLabelCommon ecpg_interval
+        | VARCHAR
         {
-            if (strlen($2) != 0 && strcmp ($1, "datetime") != 0 && strcmp ($1, "interval") != 0)
-                mmerror (PARSE_ERROR, ET_ERROR, "interval specification not allowed here");
+            $$.type_enum = ECPGt_varchar;
+            $$.type_str = EMPTY; /*mm_strdup("varchar");*/
+            $$.type_dimension = mm_strdup("-1");
+            $$.type_index = mm_strdup("-1");
+            $$.type_sizeof = NULL;
+        }
+        | FLOAT_P
+        {
+            /* Note: DOUBLE is handled in simple_type */
+            $$.type_enum = ECPGt_float;
+            $$.type_str = mm_strdup("float");
+            $$.type_dimension = mm_strdup("-1");
+            $$.type_index = mm_strdup("-1");
+            $$.type_sizeof = NULL;
+        }
+        | NUMERIC
+        {
+            $$.type_enum = ECPGt_numeric;
+            $$.type_str = mm_strdup("numeric");
+            $$.type_dimension = mm_strdup("-1");
+            $$.type_index = mm_strdup("-1");
+            $$.type_sizeof = NULL;
+        }
+        | DECIMAL_P
+        {
+            $$.type_enum = ECPGt_decimal;
+            $$.type_str = mm_strdup("decimal");
+            $$.type_dimension = mm_strdup("-1");
+            $$.type_index = mm_strdup("-1");
+            $$.type_sizeof = NULL;
+        }
+        | TIMESTAMP
+        {
+            $$.type_enum = ECPGt_timestamp;
+            $$.type_str = mm_strdup("timestamp");
+            $$.type_dimension = mm_strdup("-1");
+            $$.type_index = mm_strdup("-1");
+            $$.type_sizeof = NULL;
+        }
+        | INTERVAL ecpg_interval
+        {
+            $$.type_enum = ECPGt_interval;
+            $$.type_str = mm_strdup("interval");
+            $$.type_dimension = mm_strdup("-1");
+            $$.type_index = mm_strdup("-1");
+            $$.type_sizeof = NULL;
+        }
+        | STRING
+        {
+            if (INFORMIX_MODE)
+            {
+                /* In Informix mode, "string" is automatically a typedef */
+                $$.type_enum = ECPGt_string;
+                $$.type_str = mm_strdup("char");
+                $$.type_dimension = mm_strdup("-1");
+                $$.type_index = mm_strdup("-1");
+                $$.type_sizeof = NULL;
+            }
+            else
+            {
+                /* Otherwise, legal only if user typedef'ed it */
+                struct typedefs *this = get_typedef("string", false);
+
+                $$.type_str = (this->type->type_enum == ECPGt_varchar || this->type->type_enum == ECPGt_bytea) ? EMPTY
:mm_strdup(this->name); 
+                $$.type_enum = this->type->type_enum;
+                $$.type_dimension = this->type->type_dimension;
+                $$.type_index = this->type->type_index;
+                if (this->type->type_sizeof && strlen(this->type->type_sizeof) != 0)
+                    $$.type_sizeof = this->type->type_sizeof;
+                else
+                    $$.type_sizeof = cat_str(3, mm_strdup("sizeof("), mm_strdup(this->name), mm_strdup(")"));

+                struct_member_list[struct_level] = ECPGstruct_member_dup(this->struct_member_list);
+            }
+        }
+        | IDENT ecpg_interval
+        {
             /*
-             * Check for type names that the SQL grammar treats as
-             * unreserved keywords
+             * In C parsing mode, the above SQL type names are not keywords,
+             * so they will show up here as a plain identifier, and we need
+             * this duplicate code to recognize them.
+             *
+             * Note that we also handle the type names bytea, date, and
+             * datetime here, but not above because those are not currently
+             * SQL keywords.  If they ever become so, they must gain duplicate
+             * productions above.
              */
+            if (strlen($2) != 0 && strcmp ($1, "datetime") != 0 && strcmp ($1, "interval") != 0)
+                mmerror (PARSE_ERROR, ET_ERROR, "interval specification not allowed here");
+
             if (strcmp($1, "varchar") == 0)
             {
                 $$.type_enum = ECPGt_varchar;
@@ -686,45 +790,8 @@ var_type:    simple_type
             }
             else
             {
-                /* this is for typedef'ed types */
-                struct typedefs *this = get_typedef($1);
-
-                $$.type_str = (this->type->type_enum == ECPGt_varchar || this->type->type_enum == ECPGt_bytea) ? EMPTY
:mm_strdup(this->name); 
-                $$.type_enum = this->type->type_enum;
-                $$.type_dimension = this->type->type_dimension;
-                $$.type_index = this->type->type_index;
-                if (this->type->type_sizeof && strlen(this->type->type_sizeof) != 0)
-                    $$.type_sizeof = this->type->type_sizeof;
-                else
-                    $$.type_sizeof = cat_str(3, mm_strdup("sizeof("), mm_strdup(this->name), mm_strdup(")"));
-
-                struct_member_list[struct_level] = ECPGstruct_member_dup(this->struct_member_list);
-            }
-        }
-        | STRING
-        {
-            /*
-             * It's quite horrid that ECPGColLabelCommon excludes
-             * unreserved_keyword, meaning that unreserved keywords can't be
-             * used as type names in var_type.  However, this is hard to avoid
-             * since what follows ecpgstart can be either a random SQL
-             * statement or an ECPGVarDeclaration (beginning with var_type).
-             * Pending a bright idea about how to fix that, we must
-             * special-case STRING (and any other unreserved keywords that are
-             * likely to be needed here).
-             */
-            if (INFORMIX_MODE)
-            {
-                $$.type_enum = ECPGt_string;
-                $$.type_str = mm_strdup("char");
-                $$.type_dimension = mm_strdup("-1");
-                $$.type_index = mm_strdup("-1");
-                $$.type_sizeof = NULL;
-            }
-            else
-            {
-                /* this is for typedef'ed types */
-                struct typedefs *this = get_typedef("string");
+                /* Otherwise, it must be a user-defined typedef name */
+                struct typedefs *this = get_typedef($1, false);

                 $$.type_str = (this->type->type_enum == ECPGt_varchar || this->type->type_enum == ECPGt_bytea) ? EMPTY
:mm_strdup(this->name); 
                 $$.type_enum = this->type->type_enum;
@@ -751,7 +818,7 @@ var_type:    simple_type
             {
                 /* No */

-                this = get_typedef(name);
+                this = get_typedef(name, false);
                 $$.type_str = mm_strdup(this->name);
                 $$.type_enum = this->type->type_enum;
                 $$.type_dimension = this->type->type_dimension;
@@ -1657,17 +1724,14 @@ ColLabel:  ECPGColLabel                { $$ = $1; }
         | ECPGunreserved_interval    { $$ = $1; }
         ;

-ECPGColLabel:  ECPGColLabelCommon    { $$ = $1; }
+ECPGColLabel:  ecpg_ident            { $$ = $1; }
         | unreserved_keyword        { $$ = $1; }
-        | reserved_keyword            { $$ = $1; }
-        | ECPGKeywords_rest            { $$ = $1; }
-        | CONNECTION                { $$ = mm_strdup("connection"); }
-        ;
-
-ECPGColLabelCommon:  ecpg_ident        { $$ = $1; }
         | col_name_keyword            { $$ = $1; }
         | type_func_name_keyword    { $$ = $1; }
+        | reserved_keyword            { $$ = $1; }
         | ECPGKeywords_vanames        { $$ = $1; }
+        | ECPGKeywords_rest            { $$ = $1; }
+        | CONNECTION                { $$ = mm_strdup("connection"); }
         ;

 ECPGCKeywords: S_AUTO                { $$ = mm_strdup("auto"); }
diff --git a/src/interfaces/ecpg/preproc/ecpg.type b/src/interfaces/ecpg/preproc/ecpg.type
index d1cde691c0..4fe80a5a4b 100644
--- a/src/interfaces/ecpg/preproc/ecpg.type
+++ b/src/interfaces/ecpg/preproc/ecpg.type
@@ -3,7 +3,6 @@
 %type <str> ECPGCKeywords
 %type <str> ECPGColId
 %type <str> ECPGColLabel
-%type <str> ECPGColLabelCommon
 %type <str> ECPGConnect
 %type <str> ECPGCursorStmt
 %type <str> ECPGDeallocateDescr
diff --git a/src/interfaces/ecpg/preproc/pgc.l b/src/interfaces/ecpg/preproc/pgc.l
index 996718cb8a..c344f8f30f 100644
--- a/src/interfaces/ecpg/preproc/pgc.l
+++ b/src/interfaces/ecpg/preproc/pgc.l
@@ -983,10 +983,19 @@ cppline            {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
                     {
                         int        kwvalue;

-                        /* Is it an SQL/ECPG keyword? */
-                        kwvalue = ScanECPGKeywordLookup(yytext);
-                        if (kwvalue >= 0)
-                            return kwvalue;
+                        /*
+                         * User-defined typedefs override SQL keywords, but
+                         * not C keywords.  Currently, a typedef name is just
+                         * reported as IDENT, but someday we might need to
+                         * return a distinct token type.
+                         */
+                        if (get_typedef(yytext, true) == NULL)
+                        {
+                            /* Is it an SQL/ECPG keyword? */
+                            kwvalue = ScanECPGKeywordLookup(yytext);
+                            if (kwvalue >= 0)
+                                return kwvalue;
+                        }

                         /* Is it a C keyword? */
                         kwvalue = ScanCKeywordLookup(yytext);
diff --git a/src/interfaces/ecpg/preproc/preproc_extern.h b/src/interfaces/ecpg/preproc/preproc_extern.h
index 992797b8bb..6be59b7193 100644
--- a/src/interfaces/ecpg/preproc/preproc_extern.h
+++ b/src/interfaces/ecpg/preproc/preproc_extern.h
@@ -93,7 +93,7 @@ extern void add_variable_to_head(struct arguments **, struct variable *, struct
 extern void add_variable_to_tail(struct arguments **, struct variable *, struct variable *);
 extern void remove_variable_from_list(struct arguments **list, struct variable *var);
 extern void dump_variables(struct arguments *, int);
-extern struct typedefs *get_typedef(char *);
+extern struct typedefs *get_typedef(const char *name, bool noerror);
 extern void adjust_array(enum ECPGttype, char **, char **, char *, char *, int, bool);
 extern void reset_variables(void);
 extern void check_indicator(struct ECPGtype *);
diff --git a/src/interfaces/ecpg/preproc/variable.c b/src/interfaces/ecpg/preproc/variable.c
index 887d479e73..2a2b953118 100644
--- a/src/interfaces/ecpg/preproc/variable.c
+++ b/src/interfaces/ecpg/preproc/variable.c
@@ -497,15 +497,20 @@ check_indicator(struct ECPGtype *var)
 }

 struct typedefs *
-get_typedef(char *name)
+get_typedef(const char *name, bool noerror)
 {
     struct typedefs *this;

-    for (this = types; this && strcmp(this->name, name) != 0; this = this->next);
-    if (!this)
+    for (this = types; this != NULL; this = this->next)
+    {
+        if (strcmp(this->name, name) == 0)
+            return this;
+    }
+
+    if (!noerror)
         mmfatal(PARSE_ERROR, "unrecognized data type name \"%s\"", name);

-    return this;
+    return NULL;
 }

 void

Re: SQL/JSON functions vs. ECPG vs. STRING as a reserved word

От
Andrew Dunstan
Дата:
On 2022-05-29 Su 16:19, Tom Lane wrote:
> More generally, I feel like we have a process problem: there needs to
> be a higher bar to adding new fully- or even partially-reserved words.
> I might've missed it, but I don't recall that there was any discussion
> of the compatibility implications of this change.
>

Thanks for fixing this while I was away.

I did in fact raise the issue on 1 Feb, see
<https://postgr.es/m/f174a289-3274-569d-875c-2e810101df22@dunslane.net>,
but nobody responded that I recall. I guess I should have pushed the
discussion further


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com




Re: SQL/JSON functions vs. ECPG vs. STRING as a reserved word

От
Noah Misch
Дата:
On Mon, May 30, 2022 at 05:20:15PM -0400, Tom Lane wrote:

[allow EXEC SQL TYPE unreserved_keyword IS ...]

> 1. In pgc.l, if an identifier is a typedef name, ignore any possible
> keyword meaning and return it as an IDENT.  (I'd originally supposed
> that we'd want to return some new TYPEDEF token type, but that does
> not seem to be necessary right now, and adding a new token type would
> increase the patch footprint quite a bit.)
> 
> 2. In the var_type production, forget about ECPGColLabel[Common]
> and just handle the keywords we know we need, plus IDENT for the
> typedef case.  It turns out that we have to have duplicate coding
> because most of these keywords are not keywords in C lexing mode,
> so that they'll come through the IDENT path anyway when we're
> in a C rather than SQL context.  That seemed acceptable to me.
> I thought about adding them all to the C keywords list but that
> seemed likely to have undesirable side-effects, and again it'd
> bloat the patch footprint.
> 
> This fix is not without downsides.  Disabling recognition of
> keywords that match typedefs means that, for example, if you
> declare a typedef named "work" then ECPG will fail to parse
> "EXEC SQL BEGIN WORK".  So in a real sense this is just trading
> one hazard for another.  But there is an important difference:
> with this, whether your ECPG program works depends only on what
> typedef names and SQL commands are used in the program.  If
> it compiles today it'll still compile next year, whereas with
> the present implementation the addition of some new unreserved
> SQL keyword could break it.  We'd have to document this change
> for sure, and it wouldn't be something to back-patch, but it
> seems like it might be acceptable from the users' standpoint.

I agree this change is more likely to please a user than to harm a user.  The
user benefit is slim, but the patch is also slim.

> We could narrow (not eliminate) this hazard if we could get the
> typedef lookup in pgc.l to happen only when we're about to parse
> a var_type construct.  But because of Bison's lookahead behavior,
> that seems to be impossible, or at least undesirably messy
> and fragile.  But perhaps somebody else will see a way.

I don't, though I'm not much of a Bison wizard.

> Anyway, this seems like too big a change to consider for v15,
> so I'll stick this patch into the v16 CF queue.  It's only
> draft quality anyway --- lacks documentation changes and test
> cases.  There are also some coding points that could use review.
> Notably, I made the typedef lookup override SQL keywords but
> not C keywords; this is for consistency with the C-mode lookup
> rules, but is it the right thing?

That decision seems fine.  ScanCKeywordLookup() covers just twenty-six
keywords, and that list hasn't changed since 2003.  Moreover, most of them are
keywords of the C language itself, so allowing them would entailing mangling
them in the generated C to avoid C compiler errors.  Given the lack of
complaints, let's not go there.

I didn't locate any problems beyond the test and doc gaps that you mentioned,
so I've marked this Ready for Committer.



Re: SQL/JSON functions vs. ECPG vs. STRING as a reserved word

От
Tom Lane
Дата:
Noah Misch <noah@leadboat.com> writes:
> On Mon, May 30, 2022 at 05:20:15PM -0400, Tom Lane wrote:
>> [allow EXEC SQL TYPE unreserved_keyword IS ...]

> I didn't locate any problems beyond the test and doc gaps that you mentioned,
> so I've marked this Ready for Committer.

Thanks!  Here's a fleshed-out version with doc changes, plus adjustment
of preproc/type.pgc so that it exposes the existing problem.  (No code
changes from v1.)  I'll push this in a few days if there are not
objections.

            regards, tom lane

diff --git a/doc/src/sgml/ecpg.sgml b/doc/src/sgml/ecpg.sgml
index 7f8b4dd5c0..0ba1bf93a6 100644
--- a/doc/src/sgml/ecpg.sgml
+++ b/doc/src/sgml/ecpg.sgml
@@ -1483,6 +1483,10 @@ EXEC SQL END DECLARE SECTION;

     <sect4>
      <title>Typedefs</title>
+     <indexterm>
+      <primary>typedef</primary>
+      <secondary>in ECPG</secondary>
+     </indexterm>

      <para>
       Use the <literal>typedef</literal> keyword to map new types to already
@@ -1497,8 +1501,38 @@ EXEC SQL END DECLARE SECTION;
 <programlisting>
 EXEC SQL TYPE serial_t IS long;
 </programlisting>
-      This declaration does not need to be part of a declare section.
+      This declaration does not need to be part of a declare section;
+      that is, you can also write typedefs as normal C statements.
      </para>
+
+     <para>
+      Any word you declare as a typedef cannot be used as a SQL keyword
+      in <literal>EXEC SQL</literal> commands later in the same program.
+      For example, this won't work:
+<programlisting>
+EXEC SQL BEGIN DECLARE SECTION;
+    typedef int start;
+EXEC SQL END DECLARE SECTION;
+...
+EXEC SQL START TRANSACTION;
+</programlisting>
+      ECPG will report a syntax error for <literal>START
+      TRANSACTION</literal>, because it no longer
+      recognizes <literal>START</literal> as a SQL keyword,
+      only as a typedef.
+     </para>
+
+     <note>
+      <para>
+       In <productname>PostgreSQL</productname> releases before v16, use
+       of SQL keywords as typedef names was likely to result in syntax
+       errors associated with use of the typedef itself, rather than use
+       of the name as a SQL keyword.  The new behavior is less likely to
+       cause problems when an existing ECPG application is recompiled in
+       a new <productname>PostgreSQL</productname> release with new
+       keywords.
+      </para>
+     </note>
     </sect4>

     <sect4>
diff --git a/src/interfaces/ecpg/preproc/ecpg.trailer b/src/interfaces/ecpg/preproc/ecpg.trailer
index b95fc44314..fba35f6be6 100644
--- a/src/interfaces/ecpg/preproc/ecpg.trailer
+++ b/src/interfaces/ecpg/preproc/ecpg.trailer
@@ -564,8 +564,29 @@ var_type:    simple_type
             $$.type_index = mm_strdup("-1");
             $$.type_sizeof = NULL;
         }
-        | ECPGColLabelCommon '(' precision opt_scale ')'
+        | NUMERIC '(' precision opt_scale ')'
         {
+            $$.type_enum = ECPGt_numeric;
+            $$.type_str = mm_strdup("numeric");
+            $$.type_dimension = mm_strdup("-1");
+            $$.type_index = mm_strdup("-1");
+            $$.type_sizeof = NULL;
+        }
+        | DECIMAL_P '(' precision opt_scale ')'
+        {
+            $$.type_enum = ECPGt_decimal;
+            $$.type_str = mm_strdup("decimal");
+            $$.type_dimension = mm_strdup("-1");
+            $$.type_index = mm_strdup("-1");
+            $$.type_sizeof = NULL;
+        }
+        | IDENT '(' precision opt_scale ')'
+        {
+            /*
+             * In C parsing mode, NUMERIC and DECIMAL are not keywords, so
+             * they will show up here as a plain identifier, and we need
+             * this duplicate code to recognize them.
+             */
             if (strcmp($1, "numeric") == 0)
             {
                 $$.type_enum = ECPGt_numeric;
@@ -587,15 +608,98 @@ var_type:    simple_type
             $$.type_index = mm_strdup("-1");
             $$.type_sizeof = NULL;
         }
-        | ECPGColLabelCommon ecpg_interval
+        | VARCHAR
         {
-            if (strlen($2) != 0 && strcmp ($1, "datetime") != 0 && strcmp ($1, "interval") != 0)
-                mmerror (PARSE_ERROR, ET_ERROR, "interval specification not allowed here");
+            $$.type_enum = ECPGt_varchar;
+            $$.type_str = EMPTY; /*mm_strdup("varchar");*/
+            $$.type_dimension = mm_strdup("-1");
+            $$.type_index = mm_strdup("-1");
+            $$.type_sizeof = NULL;
+        }
+        | FLOAT_P
+        {
+            /* Note: DOUBLE is handled in simple_type */
+            $$.type_enum = ECPGt_float;
+            $$.type_str = mm_strdup("float");
+            $$.type_dimension = mm_strdup("-1");
+            $$.type_index = mm_strdup("-1");
+            $$.type_sizeof = NULL;
+        }
+        | NUMERIC
+        {
+            $$.type_enum = ECPGt_numeric;
+            $$.type_str = mm_strdup("numeric");
+            $$.type_dimension = mm_strdup("-1");
+            $$.type_index = mm_strdup("-1");
+            $$.type_sizeof = NULL;
+        }
+        | DECIMAL_P
+        {
+            $$.type_enum = ECPGt_decimal;
+            $$.type_str = mm_strdup("decimal");
+            $$.type_dimension = mm_strdup("-1");
+            $$.type_index = mm_strdup("-1");
+            $$.type_sizeof = NULL;
+        }
+        | TIMESTAMP
+        {
+            $$.type_enum = ECPGt_timestamp;
+            $$.type_str = mm_strdup("timestamp");
+            $$.type_dimension = mm_strdup("-1");
+            $$.type_index = mm_strdup("-1");
+            $$.type_sizeof = NULL;
+        }
+        | INTERVAL ecpg_interval
+        {
+            $$.type_enum = ECPGt_interval;
+            $$.type_str = mm_strdup("interval");
+            $$.type_dimension = mm_strdup("-1");
+            $$.type_index = mm_strdup("-1");
+            $$.type_sizeof = NULL;
+        }
+        | STRING
+        {
+            if (INFORMIX_MODE)
+            {
+                /* In Informix mode, "string" is automatically a typedef */
+                $$.type_enum = ECPGt_string;
+                $$.type_str = mm_strdup("char");
+                $$.type_dimension = mm_strdup("-1");
+                $$.type_index = mm_strdup("-1");
+                $$.type_sizeof = NULL;
+            }
+            else
+            {
+                /* Otherwise, legal only if user typedef'ed it */
+                struct typedefs *this = get_typedef("string", false);
+
+                $$.type_str = (this->type->type_enum == ECPGt_varchar || this->type->type_enum == ECPGt_bytea) ? EMPTY
:mm_strdup(this->name); 
+                $$.type_enum = this->type->type_enum;
+                $$.type_dimension = this->type->type_dimension;
+                $$.type_index = this->type->type_index;
+                if (this->type->type_sizeof && strlen(this->type->type_sizeof) != 0)
+                    $$.type_sizeof = this->type->type_sizeof;
+                else
+                    $$.type_sizeof = cat_str(3, mm_strdup("sizeof("), mm_strdup(this->name), mm_strdup(")"));

+                struct_member_list[struct_level] = ECPGstruct_member_dup(this->struct_member_list);
+            }
+        }
+        | IDENT ecpg_interval
+        {
             /*
-             * Check for type names that the SQL grammar treats as
-             * unreserved keywords
+             * In C parsing mode, the above SQL type names are not keywords,
+             * so they will show up here as a plain identifier, and we need
+             * this duplicate code to recognize them.
+             *
+             * Note that we also handle the type names bytea, date, and
+             * datetime here, but not above because those are not currently
+             * SQL keywords.  If they ever become so, they must gain duplicate
+             * productions above.
              */
+            if (strlen($2) != 0 && strcmp ($1, "datetime") != 0 && strcmp ($1, "interval") != 0)
+                mmerror (PARSE_ERROR, ET_ERROR, "interval specification not allowed here");
+
             if (strcmp($1, "varchar") == 0)
             {
                 $$.type_enum = ECPGt_varchar;
@@ -686,45 +790,8 @@ var_type:    simple_type
             }
             else
             {
-                /* this is for typedef'ed types */
-                struct typedefs *this = get_typedef($1);
-
-                $$.type_str = (this->type->type_enum == ECPGt_varchar || this->type->type_enum == ECPGt_bytea) ? EMPTY
:mm_strdup(this->name); 
-                $$.type_enum = this->type->type_enum;
-                $$.type_dimension = this->type->type_dimension;
-                $$.type_index = this->type->type_index;
-                if (this->type->type_sizeof && strlen(this->type->type_sizeof) != 0)
-                    $$.type_sizeof = this->type->type_sizeof;
-                else
-                    $$.type_sizeof = cat_str(3, mm_strdup("sizeof("), mm_strdup(this->name), mm_strdup(")"));
-
-                struct_member_list[struct_level] = ECPGstruct_member_dup(this->struct_member_list);
-            }
-        }
-        | STRING
-        {
-            /*
-             * It's quite horrid that ECPGColLabelCommon excludes
-             * unreserved_keyword, meaning that unreserved keywords can't be
-             * used as type names in var_type.  However, this is hard to avoid
-             * since what follows ecpgstart can be either a random SQL
-             * statement or an ECPGVarDeclaration (beginning with var_type).
-             * Pending a bright idea about how to fix that, we must
-             * special-case STRING (and any other unreserved keywords that are
-             * likely to be needed here).
-             */
-            if (INFORMIX_MODE)
-            {
-                $$.type_enum = ECPGt_string;
-                $$.type_str = mm_strdup("char");
-                $$.type_dimension = mm_strdup("-1");
-                $$.type_index = mm_strdup("-1");
-                $$.type_sizeof = NULL;
-            }
-            else
-            {
-                /* this is for typedef'ed types */
-                struct typedefs *this = get_typedef("string");
+                /* Otherwise, it must be a user-defined typedef name */
+                struct typedefs *this = get_typedef($1, false);

                 $$.type_str = (this->type->type_enum == ECPGt_varchar || this->type->type_enum == ECPGt_bytea) ? EMPTY
:mm_strdup(this->name); 
                 $$.type_enum = this->type->type_enum;
@@ -751,7 +818,7 @@ var_type:    simple_type
             {
                 /* No */

-                this = get_typedef(name);
+                this = get_typedef(name, false);
                 $$.type_str = mm_strdup(this->name);
                 $$.type_enum = this->type->type_enum;
                 $$.type_dimension = this->type->type_dimension;
@@ -1657,17 +1724,14 @@ ColLabel:  ECPGColLabel                { $$ = $1; }
         | ECPGunreserved_interval    { $$ = $1; }
         ;

-ECPGColLabel:  ECPGColLabelCommon    { $$ = $1; }
+ECPGColLabel:  ecpg_ident            { $$ = $1; }
         | unreserved_keyword        { $$ = $1; }
-        | reserved_keyword            { $$ = $1; }
-        | ECPGKeywords_rest            { $$ = $1; }
-        | CONNECTION                { $$ = mm_strdup("connection"); }
-        ;
-
-ECPGColLabelCommon:  ecpg_ident        { $$ = $1; }
         | col_name_keyword            { $$ = $1; }
         | type_func_name_keyword    { $$ = $1; }
+        | reserved_keyword            { $$ = $1; }
         | ECPGKeywords_vanames        { $$ = $1; }
+        | ECPGKeywords_rest            { $$ = $1; }
+        | CONNECTION                { $$ = mm_strdup("connection"); }
         ;

 ECPGCKeywords: S_AUTO                { $$ = mm_strdup("auto"); }
diff --git a/src/interfaces/ecpg/preproc/ecpg.type b/src/interfaces/ecpg/preproc/ecpg.type
index d1cde691c0..4fe80a5a4b 100644
--- a/src/interfaces/ecpg/preproc/ecpg.type
+++ b/src/interfaces/ecpg/preproc/ecpg.type
@@ -3,7 +3,6 @@
 %type <str> ECPGCKeywords
 %type <str> ECPGColId
 %type <str> ECPGColLabel
-%type <str> ECPGColLabelCommon
 %type <str> ECPGConnect
 %type <str> ECPGCursorStmt
 %type <str> ECPGDeallocateDescr
diff --git a/src/interfaces/ecpg/preproc/pgc.l b/src/interfaces/ecpg/preproc/pgc.l
index 996718cb8a..c344f8f30f 100644
--- a/src/interfaces/ecpg/preproc/pgc.l
+++ b/src/interfaces/ecpg/preproc/pgc.l
@@ -983,10 +983,19 @@ cppline            {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
                     {
                         int        kwvalue;

-                        /* Is it an SQL/ECPG keyword? */
-                        kwvalue = ScanECPGKeywordLookup(yytext);
-                        if (kwvalue >= 0)
-                            return kwvalue;
+                        /*
+                         * User-defined typedefs override SQL keywords, but
+                         * not C keywords.  Currently, a typedef name is just
+                         * reported as IDENT, but someday we might need to
+                         * return a distinct token type.
+                         */
+                        if (get_typedef(yytext, true) == NULL)
+                        {
+                            /* Is it an SQL/ECPG keyword? */
+                            kwvalue = ScanECPGKeywordLookup(yytext);
+                            if (kwvalue >= 0)
+                                return kwvalue;
+                        }

                         /* Is it a C keyword? */
                         kwvalue = ScanCKeywordLookup(yytext);
diff --git a/src/interfaces/ecpg/preproc/preproc_extern.h b/src/interfaces/ecpg/preproc/preproc_extern.h
index 992797b8bb..6be59b7193 100644
--- a/src/interfaces/ecpg/preproc/preproc_extern.h
+++ b/src/interfaces/ecpg/preproc/preproc_extern.h
@@ -93,7 +93,7 @@ extern void add_variable_to_head(struct arguments **, struct variable *, struct
 extern void add_variable_to_tail(struct arguments **, struct variable *, struct variable *);
 extern void remove_variable_from_list(struct arguments **list, struct variable *var);
 extern void dump_variables(struct arguments *, int);
-extern struct typedefs *get_typedef(char *);
+extern struct typedefs *get_typedef(const char *name, bool noerror);
 extern void adjust_array(enum ECPGttype, char **, char **, char *, char *, int, bool);
 extern void reset_variables(void);
 extern void check_indicator(struct ECPGtype *);
diff --git a/src/interfaces/ecpg/preproc/variable.c b/src/interfaces/ecpg/preproc/variable.c
index 887d479e73..2a2b953118 100644
--- a/src/interfaces/ecpg/preproc/variable.c
+++ b/src/interfaces/ecpg/preproc/variable.c
@@ -497,15 +497,20 @@ check_indicator(struct ECPGtype *var)
 }

 struct typedefs *
-get_typedef(char *name)
+get_typedef(const char *name, bool noerror)
 {
     struct typedefs *this;

-    for (this = types; this && strcmp(this->name, name) != 0; this = this->next);
-    if (!this)
+    for (this = types; this != NULL; this = this->next)
+    {
+        if (strcmp(this->name, name) == 0)
+            return this;
+    }
+
+    if (!noerror)
         mmfatal(PARSE_ERROR, "unrecognized data type name \"%s\"", name);

-    return this;
+    return NULL;
 }

 void
diff --git a/src/interfaces/ecpg/test/expected/preproc-type.c b/src/interfaces/ecpg/test/expected/preproc-type.c
index 1968a87574..9323cb66f8 100644
--- a/src/interfaces/ecpg/test/expected/preproc-type.c
+++ b/src/interfaces/ecpg/test/expected/preproc-type.c
@@ -33,20 +33,26 @@ typedef char  mmChar ;

 #line 7 "type.pgc"

-typedef short  mmSmallInt ;
+typedef short  access ;

 #line 8 "type.pgc"

 #line 8 "type.pgc"
+    /* matches an unreserved SQL keyword */
+typedef access  access_renamed ;
+
+#line 9 "type.pgc"
+
+#line 9 "type.pgc"


 /* exec sql type string is char [ 11 ] */
-#line 10 "type.pgc"
+#line 11 "type.pgc"

 typedef char string[11];

 /* exec sql type c is char reference */
-#line 13 "type.pgc"
+#line 14 "type.pgc"

 typedef char* c;

@@ -58,16 +64,16 @@ typedef char* c;


 struct TBempl {
-#line 19 "type.pgc"
+#line 20 "type.pgc"
  mmInteger idnum ;

-#line 20 "type.pgc"
+#line 21 "type.pgc"
  mmChar name [ 21 ] ;

-#line 21 "type.pgc"
- mmSmallInt accs ;
+#line 22 "type.pgc"
+ access accs ;
  } ;/* exec sql end declare section */
-#line 23 "type.pgc"
+#line 24 "type.pgc"


 int
@@ -77,41 +83,45 @@ main (void)



+






-#line 29 "type.pgc"
+#line 30 "type.pgc"
  struct TBempl empl ;

-#line 30 "type.pgc"
+#line 31 "type.pgc"
  string str ;

-#line 31 "type.pgc"
+#line 32 "type.pgc"
+ access accs_val = 320 ;
+
+#line 33 "type.pgc"
  c ptr = NULL ;

-#line 36 "type.pgc"
+#line 38 "type.pgc"
  struct varchar {
-#line 34 "type.pgc"
+#line 36 "type.pgc"
  int len ;

-#line 35 "type.pgc"
+#line 37 "type.pgc"
  char text [ 10 ] ;
  } vc ;
 /* exec sql end declare section */
-#line 37 "type.pgc"
+#line 39 "type.pgc"


   /* exec sql var vc is [ 10 ] */
-#line 39 "type.pgc"
+#line 41 "type.pgc"

   ECPGdebug (1, stderr);

   empl.idnum = 1;
   { ECPGconnect(__LINE__, 0, "ecpg1_regression" , NULL, NULL , NULL, 0); }
-#line 43 "type.pgc"
+#line 45 "type.pgc"

   if (sqlca.sqlcode)
     {
@@ -120,7 +130,7 @@ main (void)
     }

   { ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "create table empl ( idnum integer , name char ( 20 ) , accs
smallint, string1 char ( 10 ) , string2 char ( 10 ) , string3 char ( 10 ) )", ECPGt_EOIT, ECPGt_EORT);} 
-#line 51 "type.pgc"
+#line 53 "type.pgc"

   if (sqlca.sqlcode)
     {
@@ -128,8 +138,10 @@ main (void)
       exit (sqlca.sqlcode);
     }

-  { ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "insert into empl values ( 1 , 'user name' , 320 , 'first str' ,
'secondstr' , 'third str' )", ECPGt_EOIT, ECPGt_EORT);} 
-#line 58 "type.pgc"
+  { ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "insert into empl values ( 1 , 'user name' , $1  , 'first str' ,
'secondstr' , 'third str' )",  
+    ECPGt_short,&(accs_val),(long)1,(long)1,sizeof(short),
+    ECPGt_NO_INDICATOR, NULL , 0L, 0L, 0L, ECPGt_EOIT, ECPGt_EORT);}
+#line 60 "type.pgc"

   if (sqlca.sqlcode)
     {
@@ -152,7 +164,7 @@ main (void)
     ECPGt_NO_INDICATOR, NULL , 0L, 0L, 0L,
     ECPGt_varchar,&(vc),(long)10,(long)1,sizeof(struct varchar),
     ECPGt_NO_INDICATOR, NULL , 0L, 0L, 0L, ECPGt_EORT);}
-#line 68 "type.pgc"
+#line 70 "type.pgc"

   if (sqlca.sqlcode)
     {
@@ -162,7 +174,7 @@ main (void)
   printf ("id=%ld name='%s' accs=%d str='%s' ptr='%s' vc='%10.10s'\n", empl.idnum, empl.name, empl.accs, str, ptr,
vc.text);

   { ECPGdisconnect(__LINE__, "CURRENT");}
-#line 76 "type.pgc"
+#line 78 "type.pgc"


   free(ptr);
diff --git a/src/interfaces/ecpg/test/expected/preproc-type.stderr
b/src/interfaces/ecpg/test/expected/preproc-type.stderr
index 678eceff70..85e3094ed5 100644
--- a/src/interfaces/ecpg/test/expected/preproc-type.stderr
+++ b/src/interfaces/ecpg/test/expected/preproc-type.stderr
@@ -2,39 +2,41 @@
 [NO_PID]: sqlca: code: 0, state: 00000
 [NO_PID]: ECPGconnect: opening database ecpg1_regression on <DEFAULT> port <DEFAULT>
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_execute on line 50: query: create table empl ( idnum integer , name char ( 20 ) , accs smallint ,
string1char ( 10 ) , string2 char ( 10 ) , string3 char ( 10 ) ); with 0 parameter(s) on connection ecpg1_regression 
+[NO_PID]: ecpg_execute on line 52: query: create table empl ( idnum integer , name char ( 20 ) , accs smallint ,
string1char ( 10 ) , string2 char ( 10 ) , string3 char ( 10 ) ); with 0 parameter(s) on connection ecpg1_regression 
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_execute on line 50: using PQexec
+[NO_PID]: ecpg_execute on line 52: using PQexec
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_process_output on line 50: OK: CREATE TABLE
+[NO_PID]: ecpg_process_output on line 52: OK: CREATE TABLE
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_execute on line 58: query: insert into empl values ( 1 , 'user name' , 320 , 'first str' , 'second str'
,'third str' ); with 0 parameter(s) on connection ecpg1_regression 
+[NO_PID]: ecpg_execute on line 60: query: insert into empl values ( 1 , 'user name' , $1  , 'first str' , 'second str'
,'third str' ); with 1 parameter(s) on connection ecpg1_regression 
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_execute on line 58: using PQexec
+[NO_PID]: ecpg_execute on line 60: using PQexecParams
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_process_output on line 58: OK: INSERT 0 1
+[NO_PID]: ecpg_free_params on line 60: parameter 1 = 320
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_execute on line 65: query: select idnum , name , accs , string1 , string2 , string3 from empl where
idnum= $1 ; with 1 parameter(s) on connection ecpg1_regression 
+[NO_PID]: ecpg_process_output on line 60: OK: INSERT 0 1
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_execute on line 65: using PQexecParams
+[NO_PID]: ecpg_execute on line 67: query: select idnum , name , accs , string1 , string2 , string3 from empl where
idnum= $1 ; with 1 parameter(s) on connection ecpg1_regression 
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_free_params on line 65: parameter 1 = 1
+[NO_PID]: ecpg_execute on line 67: using PQexecParams
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_process_output on line 65: correctly got 1 tuples with 6 fields
+[NO_PID]: ecpg_free_params on line 67: parameter 1 = 1
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_get_data on line 65: RESULT: 1 offset: -1; array: no
+[NO_PID]: ecpg_process_output on line 67: correctly got 1 tuples with 6 fields
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_get_data on line 65: RESULT: user name            offset: -1; array: no
+[NO_PID]: ecpg_get_data on line 67: RESULT: 1 offset: -1; array: no
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_get_data on line 65: RESULT: 320 offset: -1; array: no
+[NO_PID]: ecpg_get_data on line 67: RESULT: user name            offset: -1; array: no
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_get_data on line 65: RESULT: first str  offset: -1; array: no
+[NO_PID]: ecpg_get_data on line 67: RESULT: 320 offset: -1; array: no
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_store_result on line 65: allocating memory for 1 tuples
+[NO_PID]: ecpg_get_data on line 67: RESULT: first str  offset: -1; array: no
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_get_data on line 65: RESULT: second str offset: -1; array: no
+[NO_PID]: ecpg_store_result on line 67: allocating memory for 1 tuples
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_get_data on line 65: RESULT: third str  offset: -1; array: no
+[NO_PID]: ecpg_get_data on line 67: RESULT: second str offset: -1; array: no
+[NO_PID]: sqlca: code: 0, state: 00000
+[NO_PID]: ecpg_get_data on line 67: RESULT: third str  offset: -1; array: no
 [NO_PID]: sqlca: code: 0, state: 00000
 [NO_PID]: ecpg_finish: connection ecpg1_regression closed
 [NO_PID]: sqlca: code: 0, state: 00000
diff --git a/src/interfaces/ecpg/test/preproc/type.pgc b/src/interfaces/ecpg/test/preproc/type.pgc
index 3200c91dc2..12459ccde8 100644
--- a/src/interfaces/ecpg/test/preproc/type.pgc
+++ b/src/interfaces/ecpg/test/preproc/type.pgc
@@ -5,7 +5,8 @@ EXEC SQL include ../regression;

 EXEC SQL typedef long mmInteger;
 EXEC SQL typedef char mmChar;
-EXEC SQL typedef short mmSmallInt;
+EXEC SQL typedef short access;    /* matches an unreserved SQL keyword */
+EXEC SQL typedef access access_renamed;

 exec sql type string is char[11];
 typedef char string[11];
@@ -18,7 +19,7 @@ struct TBempl
 {
   mmInteger idnum;
   mmChar name[21];
-  mmSmallInt accs;
+  access accs;
 };
 EXEC SQL END DECLARE SECTION;

@@ -28,6 +29,7 @@ main (void)
   EXEC SQL BEGIN DECLARE SECTION;
   struct TBempl empl;
   string str;
+  access accs_val = 320;
   c ptr = NULL;
   struct varchar
   {
@@ -55,7 +57,7 @@ main (void)
       exit (sqlca.sqlcode);
     }

-  EXEC SQL insert into empl values (1, 'user name', 320, 'first str', 'second str', 'third str');
+  EXEC SQL insert into empl values (1, 'user name', :accs_val, 'first str', 'second str', 'third str');
   if (sqlca.sqlcode)
     {
       printf ("insert error = %ld\n", sqlca.sqlcode);