Here is a v3, which (1) activates better error messages from bison
and (2) improves the error reporting from the scanner as well.

v4.

While adding a basic function call syntax to expressions, a noticed that it would be useful to access the "detail" field of syntax errors so as to report the name of the unknown function. This version just adds the hook (expr_yyerror_detailed) that could be called later for this purpose.

--
Fabien.
diff --git a/contrib/pgbench/exprparse.y b/contrib/pgbench/exprparse.y
index 243c6b9..0405a50 100644
--- a/contrib/pgbench/exprparse.y
+++ b/contrib/pgbench/exprparse.y
@@ -26,6 +26,10 @@ static PgBenchExpr *make_op(char operator, PgBenchExpr *lexpr,
 %expect 0
 %name-prefix="expr_yy"
 
+/* better error reporting with bison */
+%define parse.lac full
+%define parse.error verbose
+
 %union
 {
 	int64		ival;
diff --git a/contrib/pgbench/exprscan.l b/contrib/pgbench/exprscan.l
index 4c9229c..de3f09a 100644
--- a/contrib/pgbench/exprscan.l
+++ b/contrib/pgbench/exprscan.l
@@ -18,6 +18,13 @@ static YY_BUFFER_STATE scanbufhandle;
 static char *scanbuf;
 static int	scanbuflen;
 
+/* context information for error reporting */
+static char *expr_source = NULL;
+static int expr_lineno = 0;
+static char *expr_full_line = NULL;
+static char *expr_command = NULL;
+static int expr_col = 0;
+
 /* flex 2.5.4 doesn't bother with a decl for this */
 int expr_yylex(void);
 
@@ -52,29 +59,42 @@ space			[ \t\r\f]
 
 .				{
 					yycol += yyleng;
-					fprintf(stderr, "unexpected character '%s'\n", yytext);
+					syntax_error(expr_source, expr_lineno, expr_full_line, expr_command,
+								 "unexpected character", yytext, expr_col + yycol);
+					/* dead code, exit is called from syntax_error */
 					return CHAR_ERROR;
 				}
 %%
 
 void
-yyerror(const char *message)
+expr_yyerror_detailed(const char *message, const char * detail)
 {
-	/* yyline is always 1 as pgbench calls the parser for each line...
-	 * so the interesting location information is the column number */
-	fprintf(stderr, "%s at column %d\n", message, yycol);
-	/* go on to raise the error from pgbench with more information */
-	/* exit(1); */
+	syntax_error(expr_source, expr_lineno, expr_full_line, expr_command,
+				 message, detail, expr_col + yycol);
+}
+
+void yyerror(const char *message)
+{
+	expr_yyerror_detailed(message, NULL);
 }
 
 /*
  * Called before any actual parsing is done
  */
 void
-expr_scanner_init(const char *str)
+expr_scanner_init(const char *str, const char *source,
+				  const int lineno, const char *line,
+				  const char *cmd, const int ecol)
 {
 	Size	slen = strlen(str);
 
+	/* save context informations for error messages */
+	expr_source = (char *) source;
+	expr_lineno = (int) lineno;
+	expr_full_line = (char *) line;
+	expr_command = (char *) cmd;
+	expr_col = (int) ecol;
+
 	/*
 	 * Might be left over after error
 	 */
@@ -102,4 +122,9 @@ expr_scanner_finish(void)
 {
 	yy_delete_buffer(scanbufhandle);
 	pg_free(scanbuf);
+	expr_source = NULL;
+	expr_lineno = 0;
+	expr_full_line = NULL;
+	expr_command = NULL;
+	expr_col = 0;
 }
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index 706fdf5..3e50bae 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -291,6 +291,7 @@ typedef struct
 	int			type;			/* command type (SQL_COMMAND or META_COMMAND) */
 	int			argc;			/* number of command words */
 	char	   *argv[MAX_ARGS]; /* command word list */
+	int			cols[MAX_ARGS]; /* corresponding column starting from 1 */
 	PgBenchExpr *expr;			/* parsed expression */
 } Command;
 
@@ -2189,6 +2190,28 @@ parseQuery(Command *cmd, const char *raw_sql)
 	return true;
 }
 
+void syntax_error(const char *source, const int lineno,
+				  const char *line, const char *command,
+				  const char *msg, const char *more, const int column)
+{
+	fprintf(stderr, "%s:%d: %s", source, lineno, msg);
+	if (more)
+		fprintf(stderr, " (%s)", more);
+	if (column != -1)
+		fprintf(stderr, " at column %d", column);
+	fprintf(stderr, " in command \"%s\"\n", command);
+	if (line) {
+		fprintf(stderr, "%s\n", line);
+		if (column != -1) {
+			int i = 0;
+			for (i = 0; i < column - 1; i++)
+				fprintf(stderr, " ");
+			fprintf(stderr, "^ error found here\n");
+		}
+	}
+	exit(1);
+}
+
 /* Parse a command; return a Command struct, or NULL if it's a comment */
 static Command *
 process_commands(char *buf, const char *source, const int lineno)
@@ -2200,6 +2223,11 @@ process_commands(char *buf, const char *source, const int lineno)
 	char	   *p,
 			   *tok;
 
+/* error message generation helper */
+#define SYNTAX_ERROR(msg, more, col)					\
+	syntax_error(source, lineno, my_commands->line,		\
+				 my_commands->argv[0], msg, more, col)
+
 	/* Make the string buf end at the next newline */
 	if ((p = strchr(buf, '\n')) != NULL)
 		*p = '\0';
@@ -2228,11 +2256,13 @@ process_commands(char *buf, const char *source, const int lineno)
 		j = 0;
 		tok = strtok(++p, delim);
 
+		/* stop "set" argument parsing the variable name */
 		if (tok != NULL && pg_strcasecmp(tok, "set") == 0)
 			max_args = 2;
 
 		while (tok != NULL)
 		{
+			my_commands->cols[j] = tok - buf + 1;
 			my_commands->argv[j++] = pg_strdup(tok);
 			my_commands->argc++;
 			if (max_args >= 0 && my_commands->argc >= max_args)
@@ -2250,9 +2280,9 @@ process_commands(char *buf, const char *source, const int lineno)
 
 			if (my_commands->argc < 4)
 			{
-				fprintf(stderr, "%s: missing argument\n", my_commands->argv[0]);
-				exit(1);
+				SYNTAX_ERROR("missing arguments", NULL, -1);
 			}
+
 			/* argc >= 4 */
 
 			if (my_commands->argc == 4 || /* uniform without/with "uniform" keyword */
@@ -2267,41 +2297,34 @@ process_commands(char *buf, const char *source, const int lineno)
 			{
 				if (my_commands->argc < 6)
 				{
-					fprintf(stderr, "%s(%s): missing threshold argument\n", my_commands->argv[0], my_commands->argv[4]);
-					exit(1);
+					SYNTAX_ERROR("missing threshold argument", my_commands->argv[4], -1);
 				}
 				else if (my_commands->argc > 6)
 				{
-					fprintf(stderr, "%s(%s): too many arguments (extra:",
-							my_commands->argv[0], my_commands->argv[4]);
-					for (j = 6; j < my_commands->argc; j++)
-						fprintf(stderr, " %s", my_commands->argv[j]);
-					fprintf(stderr, ")\n");
-					exit(1);
+					SYNTAX_ERROR("too many arguments", my_commands->argv[4],
+								 my_commands->cols[6]);
 				}
 			}
 			else /* cannot parse, unexpected arguments */
 			{
-				fprintf(stderr, "%s: unexpected arguments (bad:", my_commands->argv[0]);
-				for (j = 4; j < my_commands->argc; j++)
-					fprintf(stderr, " %s", my_commands->argv[j]);
-				fprintf(stderr, ")\n");
-				exit(1);
+				SYNTAX_ERROR("unexpected argument", my_commands->argv[4],
+							 my_commands->cols[4]);
 			}
 		}
 		else if (pg_strcasecmp(my_commands->argv[0], "set") == 0)
 		{
 			if (my_commands->argc < 3)
 			{
-				fprintf(stderr, "%s: missing argument\n", my_commands->argv[0]);
-				exit(1);
+				SYNTAX_ERROR("missing argument", NULL, -1);
 			}
 
-			expr_scanner_init(my_commands->argv[2]);
+			expr_scanner_init(my_commands->argv[2], source, lineno,
+							  my_commands->line, my_commands->argv[0],
+							  my_commands->cols[2] - 1);
 
 			if (expr_yyparse() != 0)
 			{
-				fprintf(stderr, "%s: parse error\n", my_commands->argv[0]);
+				/* dead code: exit done from syntax_error called by yyerror */
 				exit(1);
 			}
 
@@ -2313,8 +2336,7 @@ process_commands(char *buf, const char *source, const int lineno)
 		{
 			if (my_commands->argc < 2)
 			{
-				fprintf(stderr, "%s: missing argument\n", my_commands->argv[0]);
-				exit(1);
+				SYNTAX_ERROR("missing argument", NULL, -1);
 			}
 
 			/*
@@ -2343,12 +2365,12 @@ process_commands(char *buf, const char *source, const int lineno)
 					pg_strcasecmp(my_commands->argv[2], "ms") != 0 &&
 					pg_strcasecmp(my_commands->argv[2], "s") != 0)
 				{
-					fprintf(stderr, "%s: unknown time unit '%s' - must be us, ms or s\n",
-							my_commands->argv[0], my_commands->argv[2]);
-					exit(1);
+					SYNTAX_ERROR("unknown time unit, must be us, ms or s",
+								 my_commands->argv[2], my_commands->cols[2]);
 				}
 			}
 
+			/* this should be an error?! */
 			for (j = 3; j < my_commands->argc; j++)
 				fprintf(stderr, "%s: extra argument \"%s\" ignored\n",
 						my_commands->argv[0], my_commands->argv[j]);
@@ -2357,22 +2379,19 @@ process_commands(char *buf, const char *source, const int lineno)
 		{
 			if (my_commands->argc < 3)
 			{
-				fprintf(stderr, "%s: missing argument\n", my_commands->argv[0]);
-				exit(1);
+				SYNTAX_ERROR("missing argument", NULL, -1);
 			}
 		}
 		else if (pg_strcasecmp(my_commands->argv[0], "shell") == 0)
 		{
 			if (my_commands->argc < 1)
 			{
-				fprintf(stderr, "%s: missing command\n", my_commands->argv[0]);
-				exit(1);
+				SYNTAX_ERROR("missing command", NULL, -1);
 			}
 		}
 		else
 		{
-			fprintf(stderr, "Invalid command %s\n", my_commands->argv[0]);
-			exit(1);
+			SYNTAX_ERROR("invalid command", NULL, -1);
 		}
 	}
 	else
@@ -2395,6 +2414,8 @@ process_commands(char *buf, const char *source, const int lineno)
 		}
 	}
 
+#undef SYNTAX_ERROR
+
 	return my_commands;
 }
 
diff --git a/contrib/pgbench/pgbench.h b/contrib/pgbench/pgbench.h
index 128bf11..2edb9cf 100644
--- a/contrib/pgbench/pgbench.h
+++ b/contrib/pgbench/pgbench.h
@@ -47,8 +47,14 @@ extern PgBenchExpr *expr_parse_result;
 
 extern int      expr_yyparse(void);
 extern int      expr_yylex(void);
+extern void expr_yyerror_detailed(const char *str, const char *dtl);
 extern void expr_yyerror(const char *str);
-extern void expr_scanner_init(const char *str);
+extern void expr_scanner_init(const char *str, const char *source,
+							  const int lineno, const char *line,
+							  const char *cmd, const int ecol);
+extern void syntax_error(const char* source, const int lineno, const char* line,
+						 const char* cmd, const char* msg, const char* more,
+						 const int col);
 extern void expr_scanner_finish(void);
 
 extern int64 strtoint64(const char *str);
-- 
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