Indeed. Here is a v2.
Here is a v3, which (1) activates better error messages from bison
and (2) improves the error reporting from the scanner as well.
sh> ./pgbench -f bad.sql
bad.sql:3: syntax error at column 23 in command "set"
\set aid (1021 * :id) %
^ error found here
the improved message is:
bad.sql:3: syntax error, unexpected $end at column 23 in command "set"
\set aid (1021 * :id) %
^ error found here
Also scanner errors are better:
sh> ./pgbench -f bad5.sql
bad5.sql:1: unexpected character (a) at column 12 in command "set"
\set foo 12abc
^ error found here
--
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..9de7dab 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,7 +59,9 @@ 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;
}
%%
@@ -60,21 +69,27 @@ space [ \t\r\f]
void
yyerror(const char *message)
{
- /* 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, NULL, expr_col + yycol);
}
/*
* 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 +117,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..9143a17 100644
--- a/contrib/pgbench/pgbench.h
+++ b/contrib/pgbench/pgbench.h
@@ -48,7 +48,12 @@ extern PgBenchExpr *expr_parse_result;
extern int expr_yyparse(void);
extern int expr_yylex(void);
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
