Fix ECPG's handling of type names that match SQL keywords.

tglsfdc · tglsfdc · commit 83f1c7b742e8 · 2022-07-12T17:05:46.000-04:00
Previously, ECPG could only cope with variable declarations whose type names either weren't any SQL keyword, or were at least partially reserved. If you tried to use something in the unreserved_keyword category, you got a syntax error. This is pretty awful, not only because it says right on the tin that those words are not reserved, but because the set of such keywords tends to grow over time. Thus, an ECPG program that was just fine last year could fail when recompiled with a newer SQL grammar. We had to work around this recently when STRING became a keyword, but it's time for an actual fix instead of a band-aid. To fix, borrow a trick from C parsers and make the lexer's behavior change when it sees a word that is known as a typedef. This is not free of downsides: if you try to use such a name as a SQL keyword in EXEC SQL later in the program, it won't be recognized as a SQL keyword, leading to a syntax error there instead. 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 text. If it compiles today it'll still compile next year, even if more words have become SQL keywords. Discussion: https://postgr.es/m/3661437.1653855582@sss.pgh.pa.us
diff --git 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,41 @@ 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.
+      (If you have such a conflict, and renaming the typedef
+      seems impractical, you could write the SQL command
+      using <link linkend="ecpg-dynamic">dynamic SQL</link>.)
+     </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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/src/interfaces/ecpg/test/expected/preproc-type.stderr b/src/interfaces/ecpg/test/expected/preproc-type.stderr
diff --git a/src/interfaces/ecpg/test/preproc/type.pgc b/src/interfaces/ecpg/test/preproc/type.pgc
