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 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
