Hello Tatsuo-san,

Thank you for the patch. I tested a little bit and found that it does
not allow value replacement against non ascii variables in given SQL
statements . Is it intentional?

No, this is a bug.

If not, I think you need to fix parseVariable() as well.

Indeed. Here is v2.

--
Fabien.
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 1eee8dc..502d31b 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -825,6 +825,8 @@ pgbench <optional> <replaceable>options</> </optional> <replaceable>dbname</>
      <para>
       Sets variable <replaceable>varname</> to a value calculated
       from <replaceable>expression</>.
+      The variable name must consist of letters (including non-Latin letters),
+      digits, and underscores.
       The expression may contain integer constants such as <literal>5432</>,
       double constants such as <literal>3.14159</>,
       references to variables <literal>:</><replaceable>variablename</>,
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index dc1367b..1862fe4 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -58,9 +58,9 @@ extern void expr_yyset_column(int column_no, yyscan_t yyscanner);
 %option prefix="expr_yy"
 
 /* Character classes */
-alpha			[a-zA-Z_]
+alpha			[a-zA-Z\200-\377_]
 digit			[0-9]
-alnum			[a-zA-Z0-9_]
+alnum			[A-Za-z\200-\377_0-9]
 /* {space} + {nonspace} + {newline} should cover all characters */
 space			[ \t\r\f\v]
 nonspace		[^ \t\r\f\v\n]
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index ae36247..f49e8c6 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -1020,19 +1020,35 @@ makeVariableNumeric(Variable *var)
 	return true;
 }
 
-/* check whether the name consists of alphabets, numerals and underscores. */
+/*
+ * Check whether a variable's name is allowed.
+ *
+ * We allow any non-ASCII character, as well as ASCII letters, digits, and
+ * underscore.  Keep this in sync with the definition of variable_char in
+ * psqlscan.l and psqlscanslash.l.
+ *
+ * This static function is copied from "src/bin/psql/variables.c"
+ */
 static bool
-isLegalVariableName(const char *name)
+valid_variable_name(const char *name)
 {
-	int			i;
+	const unsigned char *ptr = (const unsigned char *) name;
 
-	for (i = 0; name[i] != '\0'; i++)
+	/* Mustn't be zero-length */
+	if (*ptr == '\0')
+		return false;
+
+	while (*ptr)
 	{
-		if (!isalnum((unsigned char) name[i]) && name[i] != '_')
+		if (IS_HIGHBIT_SET(*ptr) ||
+			strchr("ABCDEFGHIJKLMNOPQRSTUVWXYZ" "abcdefghijklmnopqrstuvwxyz"
+				   "_0123456789", *ptr) != NULL)
+			ptr++;
+		else
 			return false;
 	}
 
-	return (i > 0);				/* must be non-empty */
+	return true;
 }
 
 /*
@@ -1054,7 +1070,7 @@ lookupCreateVariable(CState *st, const char *context, char *name)
 		 * Check for the name only when declaring a new variable to avoid
 		 * overhead.
 		 */
-		if (!isLegalVariableName(name))
+		if (!valid_variable_name(name))
 		{
 			fprintf(stderr, "%s: invalid variable name: \"%s\"\n",
 					context, name);
@@ -1148,7 +1164,7 @@ parseVariable(const char *sql, int *eaten)
 	do
 	{
 		i++;
-	} while (isalnum((unsigned char) sql[i]) || sql[i] == '_');
+	} while (IS_HIGHBIT_SET(sql[i]) || isalnum((unsigned char) sql[i]) || sql[i] == '_');
 	if (i == 1)
 		return NULL;
 
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to