Re: improve pgbench syntax error messages

On Sat, Mar 7, 2015 at 5:49 AM, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)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 verbose

What 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