improve pgbench syntax error messages
Started by Fabien COELHOalmost 11 years ago10 messages
Report the origin of syntax errors from pgbench.
Currently only the column number (for expressions) and command are
essentially reported:
sh> ./pgbench -f bad.sql
syntax error at column 14
set: parse error
The patch helps locate the origin of errors with the file name, line
number and the actual text triggering the issue (either the line or an
extract for expressions):
sh> ./pgbench -f bad.sql
syntax error at column 14
error while processing "bad.sql" line 3: (1021 * :id) %
set: parse error
Whether using a macro is the right tool is debatable. The contents could
be expanded, but that would mean replicating the same message over and
over again, so it seems cleaner to me this way. An function seems
overkill.
--
Fabien.
Attachments:
pgbench-expr-error-1.patchtext/x-diff; name=pgbench-expr-error-1.patchDownload
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index 706fdf5..f45d28b 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -2200,6 +2200,11 @@ process_commands(char *buf, const char *source, const int lineno)
char *p,
*tok;
+/* error message generation */
+#define PRINT_ERROR_AT(current_line) \
+ fprintf(stderr, "error while processing \"%s\" line %d: %s\n", \
+ source, lineno, current_line)
+
/* Make the string buf end at the next newline */
if ((p = strchr(buf, '\n')) != NULL)
*p = '\0';
@@ -2250,6 +2255,7 @@ process_commands(char *buf, const char *source, const int lineno)
if (my_commands->argc < 4)
{
+ PRINT_ERROR_AT(my_commands->line);
fprintf(stderr, "%s: missing argument\n", my_commands->argv[0]);
exit(1);
}
@@ -2267,11 +2273,13 @@ process_commands(char *buf, const char *source, const int lineno)
{
if (my_commands->argc < 6)
{
+ PRINT_ERROR_AT(my_commands->line);
fprintf(stderr, "%s(%s): missing threshold argument\n", my_commands->argv[0], my_commands->argv[4]);
exit(1);
}
else if (my_commands->argc > 6)
{
+ PRINT_ERROR_AT(my_commands->line);
fprintf(stderr, "%s(%s): too many arguments (extra:",
my_commands->argv[0], my_commands->argv[4]);
for (j = 6; j < my_commands->argc; j++)
@@ -2282,6 +2290,7 @@ process_commands(char *buf, const char *source, const int lineno)
}
else /* cannot parse, unexpected arguments */
{
+ PRINT_ERROR_AT(my_commands->line);
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]);
@@ -2293,6 +2302,7 @@ process_commands(char *buf, const char *source, const int lineno)
{
if (my_commands->argc < 3)
{
+ PRINT_ERROR_AT(my_commands->line);
fprintf(stderr, "%s: missing argument\n", my_commands->argv[0]);
exit(1);
}
@@ -2301,6 +2311,7 @@ process_commands(char *buf, const char *source, const int lineno)
if (expr_yyparse() != 0)
{
+ PRINT_ERROR_AT(my_commands->argv[2]);
fprintf(stderr, "%s: parse error\n", my_commands->argv[0]);
exit(1);
}
@@ -2313,6 +2324,7 @@ process_commands(char *buf, const char *source, const int lineno)
{
if (my_commands->argc < 2)
{
+ PRINT_ERROR_AT(my_commands->line);
fprintf(stderr, "%s: missing argument\n", my_commands->argv[0]);
exit(1);
}
@@ -2343,6 +2355,7 @@ 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)
{
+ PRINT_ERROR_AT(my_commands->line);
fprintf(stderr, "%s: unknown time unit '%s' - must be us, ms or s\n",
my_commands->argv[0], my_commands->argv[2]);
exit(1);
@@ -2357,6 +2370,7 @@ process_commands(char *buf, const char *source, const int lineno)
{
if (my_commands->argc < 3)
{
+ PRINT_ERROR_AT(my_commands->line);
fprintf(stderr, "%s: missing argument\n", my_commands->argv[0]);
exit(1);
}
@@ -2365,12 +2379,14 @@ process_commands(char *buf, const char *source, const int lineno)
{
if (my_commands->argc < 1)
{
+ PRINT_ERROR_AT(my_commands->line);
fprintf(stderr, "%s: missing command\n", my_commands->argv[0]);
exit(1);
}
}
else
{
+ PRINT_ERROR_AT(my_commands->line);
fprintf(stderr, "Invalid command %s\n", my_commands->argv[0]);
exit(1);
}
@@ -2395,6 +2411,8 @@ process_commands(char *buf, const char *source, const int lineno)
}
}
+#undef PRINT_ERROR_AT
+
return my_commands;
}
Re: improve pgbench syntax error messages
On Tue, Mar 3, 2015 at 3:48 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
Report the origin of syntax errors from pgbench.
Currently only the column number (for expressions) and command are
essentially reported:sh> ./pgbench -f bad.sql
syntax error at column 14
set: parse errorThe patch helps locate the origin of errors with the file name, line number
and the actual text triggering the issue (either the line or an extract for
expressions):sh> ./pgbench -f bad.sql
syntax error at column 14
error while processing "bad.sql" line 3: (1021 * :id) %
set: parse errorWhether using a macro is the right tool is debatable. The contents could be
expanded, but that would mean replicating the same message over and over
again, so it seems cleaner to me this way. An function seems overkill.
As I mentioned on the other thread, I'd really like to get this into a
better format, where each error message is on one line. Looking at
that, you can't tell whether you've got one mistake, two mistakes, or
three mistakes.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: improve pgbench syntax error messages
As I mentioned on the other thread, I'd really like to get this into a
better format, where each error message is on one line. Looking at
that, you can't tell whether you've got one mistake, two mistakes, or
three mistakes.
Indeed. Here is a v2.
sh> ./pgbench -f bad.sql
bad.sql:3: syntax error at column 23 in command "set"
\set aid (1021 * :id) %
^ error found here
sh> ./pgbench -f bad2.sql
bad2.sql:1: unexpected argument (x) at column 25 in command "setrandom"
\setrandom id 1 1000 x
^ error found here
sh> ./pgbench -f bad3.sql
bad3.sql:1: too many arguments (gaussian) at column 35 in command "setrandom"
\setrandom foo 1 10 gaussian 10.3 1
^ error found here
sh> ./pgbench -f bad4.sql
bad4.sql:1: missing threshold argument (exponential) in command "setrandom"
\setrandom foo 1 10 exponential
I think that transforming unexpected garbage in errors would be a good
move, even if this breaks backwards compatibility (currently a warning is
printed about ignored extra stuff).
--
Fabien.
Attachments:
pgbench-expr-error-2.patchtext/x-diff; name=pgbench-expr-error-2.patchDownload
diff --git a/contrib/pgbench/exprscan.l b/contrib/pgbench/exprscan.l
index 4c9229c..dda2635 100644
--- a/contrib/pgbench/exprscan.l
+++ b/contrib/pgbench/exprscan.l
@@ -57,24 +57,36 @@ space [ \t\r\f]
}
%%
+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;
+
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 +114,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);
Re: improve pgbench syntax error messages
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.
Attachments:
pgbench-expr-error-3.patchtext/x-diff; name=pgbench-expr-error-3.patchDownload
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);
Re: improve pgbench syntax error messages
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.
Attachments:
pgbench-expr-error-4.patchtext/x-diff; name=pgbench-expr-error-4.patchDownload
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);
Re: improve pgbench syntax error messages
On Sat, Mar 7, 2015 at 5:49 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
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.
Let's not add code that isn't doing anything yet. We can patch the
system more later if we need to.
+/* better error reporting with bison */
+%define parse.lac full
+%define parse.error verboseWhat does this do? The comments should explain, I think. Does it
work on all versions of bison >= the minimum version we support?
+void syntax_error(const char *source, const int lineno,
Not project style.
+ if (line) {
That isn't either.
How about having syntax_error using appendPQExpBuffer() and friends
instead of printing data one character at a time?
+/* error message generation helper */
+#define SYNTAX_ERROR(msg, more, col) \
+ syntax_error(source, lineno, my_commands->line, \
+ my_commands->argv[0], msg, more, col)
+I don't like this. Just substitute the expansion. Otherwise there's
a non-obvious dependency on my_commands.
+ /* stop "set" argument parsing the variable name */
This comment addition isn't related to the purpose of this patch, and
I also can't understand what the new comment is supposed to mean.
It's the value we don't want to strtok()-ify, not the name.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: improve pgbench syntax error messages
Hello,
Here is a v5.
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.Let's not add code that isn't doing anything yet. We can patch the
system more later if we need to.
Ok.
+/* better error reporting with bison */ +%define parse.lac full +%define parse.error verboseWhat does this do?
It adds an explanation on syntax errors, instead of the laconic "syntax
error" which is akin to a boolean.
The comments should explain, I think.
I thought it did with the sentence "better error reporting with bison".
Does it work on all versions of bison >= the minimum version we support?
Good question. After some digging, it seems to be 1.85 since pg 7.4... It
does not seem to have a '%define' directive yet. That's too bad, because I
like the better messages. So I put the define in a comment.
Maybe some conditional activation from configure generated macro could be
possible, but I did not find anything about bison version.
+void syntax_error(const char *source, const int lineno, + if (line) {Not project style.
Indeed.
How about having syntax_error using appendPQExpBuffer() and friends
instead of printing data one character at a time?
This is just a basic error message printed before exiting. The simpler and
more straightforward the better, IMHO. Pgbench relies on basic
"fprintf(stderr, ...)" for error messages everywhere, I tried to blend in,
eg I avoided "fputc" for printing alignment spaces for instance. PQ buffer
stuff is not used anywhere else in pgbench. It seems to me overkill to
introduce it there just for this function. I'll do it only if required.
+/* error message generation helper */ +#define SYNTAX_ERROR(msg, more, col) \ + syntax_error(source, lineno, my_commands->line, \ + my_commands->argv[0], msg, more, col) +I don't like this. Just substitute the expansion. Otherwise there's
a non-obvious dependency on my_commands.
I wanted to have one line calls and to avoid repeating the same list of
arguments over and over again, especially with the very long "my_commands"
variable name.
Once expanded, the result is either two long lines or three
under-80-column lines per call. Not sure which one of these ugly choices
is best. I took the first option.
+ /* stop "set" argument parsing the variable name */
This comment addition isn't related to the purpose of this patch, and
I also can't understand what the new comment is supposed to mean.
It's the value we don't want to strtok()-ify, not the name.
I wanted to write: "stop set argument parsing after the variable name"
(I forgot "after") because I had to figure out the max_args management
and a minimal comment would have helped. Maybe not with this comment
though. I removed it.
--
Fabien.
Attachments:
pgbench-expr-error-5.patchtext/x-diff; name=pgbench-expr-error-5.patchDownload
diff --git a/contrib/pgbench/exprparse.y b/contrib/pgbench/exprparse.y
index 243c6b9..7173448 100644
--- a/contrib/pgbench/exprparse.y
+++ b/contrib/pgbench/exprparse.y
@@ -26,6 +26,13 @@ static PgBenchExpr *make_op(char operator, PgBenchExpr *lexpr,
%expect 0
%name-prefix="expr_yy"
+/*
+// improve bison generated syntax error messages
+// *but* not available with PostgreSQL mimimal bison 1.875
+%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..ed3c786 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,29 @@ 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 != NULL)
+ fprintf(stderr, " (%s)", more);
+ if (column != -1)
+ fprintf(stderr, " at column %d", column);
+ fprintf(stderr, " in command \"%s\"\n", command);
+ if (line != NULL) {
+ fprintf(stderr, "%s\n", line);
+ if (column != -1) {
+ int i;
+ 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)
@@ -2233,6 +2257,7 @@ process_commands(char *buf, const char *source, const int lineno)
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 +2275,10 @@ 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(source, lineno, my_commands->line, my_commands->argv[0],
+ "missing arguments", NULL, -1);
}
+
/* argc >= 4 */
if (my_commands->argc == 4 || /* uniform without/with "uniform" keyword */
@@ -2267,41 +2293,38 @@ 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(source, lineno, my_commands->line, my_commands->argv[0],
+ "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(source, lineno, my_commands->line, my_commands->argv[0],
+ "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(source, lineno, my_commands->line, my_commands->argv[0],
+ "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(source, lineno, my_commands->line, my_commands->argv[0],
+ "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,8 @@ 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(source, lineno, my_commands->line, my_commands->argv[0],
+ "missing argument", NULL, -1);
}
/*
@@ -2343,12 +2366,13 @@ 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(source, lineno, my_commands->line, my_commands->argv[0],
+ "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 +2381,22 @@ 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(source, lineno, my_commands->line, my_commands->argv[0],
+ "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(source, lineno, my_commands->line, my_commands->argv[0],
+ "missing command", NULL, -1);
}
}
else
{
- fprintf(stderr, "Invalid command %s\n", my_commands->argv[0]);
- exit(1);
+ syntax_error(source, lineno, my_commands->line, my_commands->argv[0],
+ "invalid command", NULL, -1);
}
}
else
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);
Re: improve pgbench syntax error messages
Here is a v5.
Here is v6, just a rebase.
--
Fabien.
Attachments:
pgbench-expr-error-6.patchtext/x-diff; name=pgbench-expr-error-6.patchDownload
diff --git a/contrib/pgbench/exprparse.y b/contrib/pgbench/exprparse.y
index e68631e..68c85c9 100644
--- a/contrib/pgbench/exprparse.y
+++ b/contrib/pgbench/exprparse.y
@@ -26,6 +26,13 @@ static PgBenchExpr *make_op(char operator, PgBenchExpr *lexpr,
%expect 0
%name-prefix="expr_yy"
+/*
+// improve bison generated syntax error messages
+// *but* not available with PostgreSQL mimimal bison 1.875
+%define parse.lac full
+%define parse.error verbose
+*/
+
%union
{
int64 ival;
diff --git a/contrib/pgbench/exprscan.l b/contrib/pgbench/exprscan.l
index 6e3331d..5331ab7 100644
--- a/contrib/pgbench/exprscan.l
+++ b/contrib/pgbench/exprscan.l
@@ -17,6 +17,13 @@ static int yyline = 0, yycol = 0;
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;
%}
%option 8bit
@@ -56,7 +63,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;
}
%%
@@ -64,20 +73,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 */
+ 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
*/
@@ -105,4 +121,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 822adfd..6a4805e 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -287,6 +287,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;
@@ -2185,6 +2186,29 @@ 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 != NULL)
+ fprintf(stderr, " (%s)", more);
+ if (column != -1)
+ fprintf(stderr, " at column %d", column);
+ fprintf(stderr, " in command \"%s\"\n", command);
+ if (line != NULL) {
+ fprintf(stderr, "%s\n", line);
+ if (column != -1) {
+ int i;
+ 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)
@@ -2229,6 +2253,7 @@ process_commands(char *buf, const char *source, const int lineno)
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)
@@ -2246,9 +2271,10 @@ 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(source, lineno, my_commands->line, my_commands->argv[0],
+ "missing arguments", NULL, -1);
}
+
/* argc >= 4 */
if (my_commands->argc == 4 || /* uniform without/with "uniform" keyword */
@@ -2263,41 +2289,38 @@ 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(source, lineno, my_commands->line, my_commands->argv[0],
+ "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(source, lineno, my_commands->line, my_commands->argv[0],
+ "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(source, lineno, my_commands->line, my_commands->argv[0],
+ "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(source, lineno, my_commands->line, my_commands->argv[0],
+ "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);
}
@@ -2309,8 +2332,8 @@ 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(source, lineno, my_commands->line, my_commands->argv[0],
+ "missing argument", NULL, -1);
}
/*
@@ -2339,12 +2362,13 @@ 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(source, lineno, my_commands->line, my_commands->argv[0],
+ "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]);
@@ -2353,22 +2377,22 @@ 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(source, lineno, my_commands->line, my_commands->argv[0],
+ "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(source, lineno, my_commands->line, my_commands->argv[0],
+ "missing command", NULL, -1);
}
}
else
{
- fprintf(stderr, "Invalid command %s\n", my_commands->argv[0]);
- exit(1);
+ syntax_error(source, lineno, my_commands->line, my_commands->argv[0],
+ "invalid command", NULL, -1);
}
}
else
diff --git a/contrib/pgbench/pgbench.h b/contrib/pgbench/pgbench.h
index 0396e55..a3db6b9 100644
--- a/contrib/pgbench/pgbench.h
+++ b/contrib/pgbench/pgbench.h
@@ -47,7 +47,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);
Re: improve pgbench syntax error messages
On Sun, Mar 29, 2015 at 2:20 PM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
Here is a v5.
Here is v6, just a rebase.
Committed with minor stylistic fixes.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: improve pgbench syntax error messages
Here is v6, just a rebase.
Committed with minor stylistic fixes.
Thanks!
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers