Improve pgbench error reporting.

This would have been worth doing on general principle anyway, but the
recent addition of an expression syntax to pgbench makes it an even
better idea than it would have been otherwise.

Fabien Coelho

diff --git a/contrib/pgbench/exprscan.l b/contrib/pgbench/exprscan.l
index 6e3331d908e02b9d5ad4d01435490e6577822a2e..5331ab778b4c168ab57f7363083933b4c3237b91 100644 (file)
--- 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 321a6dbdc7f80f3c9cd46206869622d63ebed078..788484e8793eb30602d88bb6c8caa963aabf626a 100644 (file)
--- 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,32 @@ 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 +2256,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 +2274,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 +2292,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 +2335,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 +2365,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 +2380,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 0396e551f24042eadea1f2d6757f9dda9a18343e..a3db6b97cc95b0939a2bb26fb5cc7931cb3da0b0 100644 (file)
--- 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);