Dear patchers,
Please find attached a patch to fix the "CREATE FUNCTION" syntax error
position bug I reported a few days ago.
As the exact query being processed in only known to the backend (it may be
the initial query, it may be a subset of the initial query, it may be some
generated query?), the offending string is returned with the error
position, which is expressed with respect to this query (that has always
been the case by the way).
In order to do that, I did the following:
1. appended a new "query" field into the ErrorData structure,
which is set with an added errquery function.
2. modified the error reporting part of the protocol (version 3).
As the protocol implementation in libpq is fuzzy enough, there is
not fix on the client reception part, only the server sending
part is modified with a new field for the query (Q). Thus this
addition should not harm old clients.
3. fixed yyerror to return the processed query on errors.
a copy of the buffer is needed as the scanner buffer is modified
and the convention about buffer termination are not the same.
4. fixed the psql position reporting code to use this reported query
instead of the one it sent. Tom's quick fix around the problem is removed.
5. updated comments where it seemd appropriate in the code.
6. finally updated the protocol documentation for the error reporting
part by adding the Q field and fixing the P field.
I dit it like that because I don't think it is elegantly feasible to
update the position to get back to the initial query, as escapes may have
been processed within the string, so a simple offset would not fix the
bug.
It validates for me.
Have a nice day,
--
Fabien Coelho - [EMAIL PROTECTED]
*** ./doc/src/sgml/protocol.sgml.orig Wed Mar 10 15:56:59 2004
--- ./doc/src/sgml/protocol.sgml Thu Mar 18 10:19:24 2004
***************
*** 3890,3901 ****
<VarListEntry>
<Term>
<literal>P</>
</Term>
<ListItem>
<Para>
Position: the field value is a decimal ASCII integer, indicating
! an error cursor position as an index into the original query string.
The first character has index 1, and positions are measured in
characters not bytes.
</Para>
--- 3890,3916 ----
<VarListEntry>
<Term>
+ <literal>Q</>
+ </Term>
+ <ListItem>
+ <Para>
+ Query: an optional string that contains the processed query string
+ on syntax errors. The position field is expressed with respect to
+ this string. In most cases this the the initial query sent by
+ the client, however in some cases such as with
+ <command>CREATE FUNCTION</> this may be a subset of the initial query.
+ </Para>
+ </ListItem>
+ </VarListEntry>
+
+ <VarListEntry>
+ <Term>
<literal>P</>
</Term>
<ListItem>
<Para>
Position: the field value is a decimal ASCII integer, indicating
! an error cursor position as an index into the processed query string.
The first character has index 1, and positions are measured in
characters not bytes.
</Para>
*** ./src/backend/parser/scan.l.orig Tue Feb 24 22:45:18 2004
--- ./src/backend/parser/scan.l Thu Mar 18 10:09:57 2004
***************
*** 68,73 ****
--- 68,74 ----
/* Handles to the buffer that the lexer uses internally */
static YY_BUFFER_STATE scanbufhandle;
static char *scanbuf;
+ static char *initial_scanbuf; /* for syntax errors, as scanbuf is touched */
unsigned char unescape_single_char(unsigned char c);
***************
*** 623,628 ****
--- 624,630 ----
(errcode(ERRCODE_SYNTAX_ERROR),
/* translator: %s is typically "syntax error" */
errmsg("%s at end of input", message),
+ errquery(initial_scanbuf),
errposition(cursorpos)));
}
else
***************
*** 631,636 ****
--- 633,639 ----
(errcode(ERRCODE_SYNTAX_ERROR),
/* translator: first %s is typically "syntax error" */
errmsg("%s at or near \"%s\"", message, loc),
+ errquery(initial_scanbuf),
errposition(cursorpos)));
}
}
***************
*** 658,663 ****
--- 661,671 ----
scanbuf[slen] = scanbuf[slen + 1] = YY_END_OF_BUFFER_CHAR;
scanbufhandle = yy_scan_buffer(scanbuf, slen + 2);
+ /*
+ * Make a copy in case of error, as scanbuf may be touched.
+ */
+ initial_scanbuf = pstrdup(str);
+
/* initialize literal buffer to a reasonable but expansible size */
literalalloc = 128;
literalbuf = (char *) palloc(literalalloc);
***************
*** 675,680 ****
--- 683,689 ----
{
yy_delete_buffer(scanbufhandle);
pfree(scanbuf);
+ pfree(initial_scanbuf);
}
*** ./src/backend/utils/error/elog.c.orig Mon Mar 15 17:57:28 2004
--- ./src/backend/utils/error/elog.c Thu Mar 18 09:51:24 2004
***************
*** 111,116 ****
--- 111,117 ----
char *detail; /* detail error message */
char *hint; /* hint message */
char *context; /* context message */
+ char *query; /* offending query string */
int cursorpos; /* cursor index into query
string */
int saved_errno; /* errno at entry */
} ErrorData;
***************
*** 793,798 ****
--- 794,820 ----
}
/*
+ * errquery --- give offending query, as it may not be exactly the one send.
+ */
+ int
+ errquery(const char * query)
+ {
+ ErrorData *edata = &errordata[errordata_stack_depth];
+ MemoryContext oldcontext;
+
+ recursion_depth++;
+ CHECK_STACK_DEPTH();
+ oldcontext = MemoryContextSwitchTo(ErrorContext);
+
+ edata->query = pstrdup(query);
+
+ MemoryContextSwitchTo(oldcontext);
+ recursion_depth--;
+
+ return 0; /* return value does not
matter */
+ }
+
+ /*
* errposition --- add cursor position to the current error
*/
int
***************
*** 1353,1358 ****
--- 1375,1386 ----
pq_sendstring(&msgbuf, edata->context);
}
+ if (edata->query)
+ {
+ pq_sendbyte(&msgbuf, PG_DIAG_STATEMENT);
+ pq_sendstring(&msgbuf, edata->query);
+ }
+
if (edata->cursorpos > 0)
{
snprintf(tbuf, sizeof(tbuf), "%d", edata->cursorpos);
*** ./src/bin/psql/common.c.orig Mon Mar 15 16:13:12 2004
--- ./src/bin/psql/common.c Thu Mar 18 09:55:11 2004
***************
*** 347,381 ****
/*
* on errors, print syntax error position if available.
- *
- * the query is expected to be in the client encoding.
*/
static void
! ReportSyntaxErrorPosition(const PGresult *result, const char *query)
{
#define DISPLAY_SIZE 60 /* screen width limit, in screen cols */
#define MIN_RIGHT_CUT 10 /* try to keep this far away from EOL */
int loc = 0;
! const char *sp;
int clen, slen, i, *qidx, *scridx, qoffset, scroffset, ibeg, iend,
loc_line;
char *wquery;
bool beg_trunc, end_trunc;
PQExpBufferData msg;
if (query == NULL)
! return; /* nothing to do */
sp = PQresultErrorField(result, PG_DIAG_STATEMENT_POSITION);
if (sp == NULL)
return; /* no syntax error location */
- /*
- * We punt if the report contains any CONTEXT. This typically means that
- * the syntax error is from inside a function, and the cursor position
- * is not relevant to the original query string.
- */
- if (PQresultErrorField(result, PG_DIAG_CONTEXT) != NULL)
- return;
if (sscanf(sp, "%d", &loc) != 1)
{
--- 347,382 ----
/*
* on errors, print syntax error position if available.
*/
static void
! ReportSyntaxErrorPosition(const PGresult *result)
{
#define DISPLAY_SIZE 60 /* screen width limit, in screen cols */
#define MIN_RIGHT_CUT 10 /* try to keep this far away from EOL */
int loc = 0;
! const char *sp, *query;
int clen, slen, i, *qidx, *scridx, qoffset, scroffset, ibeg, iend,
loc_line;
char *wquery;
bool beg_trunc, end_trunc;
PQExpBufferData msg;
+ /* get the query from the error report.
+ *
+ * indeed, although most of the time the query processed was the
+ * query passed by the client, which is still available to the client,
+ * in some cases such as with CREATE FUNCTION the query processed
+ * is only a part of the initial query sent. As the position is given
+ * wrt the query being processed, this one must be given by the backend.
+ */
+ query = PQresultErrorField(result, PG_DIAG_STATEMENT);
if (query == NULL)
! return; /* no query, nothing to do */
!
sp = PQresultErrorField(result, PG_DIAG_STATEMENT_POSITION);
if (sp == NULL)
return; /* no syntax error location */
if (sscanf(sp, "%d", &loc) != 1)
{
***************
*** 384,389 ****
--- 385,394 ----
return;
}
+ /* It would be great to have the length of the offending token,
+ * so that highlighting may do some different jobs.
+ */
+
/* Make a writable copy of the query, and a buffer for messages. */
wquery = pg_strdup(query);
***************
*** 565,571 ****
* Returns true for valid result, false for error state.
*/
static bool
! AcceptResult(const PGresult *result, const char *query)
{
bool OK = true;
--- 570,576 ----
* Returns true for valid result, false for error state.
*/
static bool
! AcceptResult(const PGresult *result)
{
bool OK = true;
***************
*** 596,602 ****
if (!OK)
{
psql_error("%s", PQerrorMessage(pset.db));
! ReportSyntaxErrorPosition(result, query);
CheckConnection();
}
--- 601,607 ----
if (!OK)
{
psql_error("%s", PQerrorMessage(pset.db));
! ReportSyntaxErrorPosition(result);
CheckConnection();
}
***************
*** 660,666 ****
res = PQexec(pset.db, query);
! if (!AcceptResult(res, query) && res)
{
PQclear(res);
res = NULL;
--- 665,671 ----
res = PQexec(pset.db, query);
! if (!AcceptResult(res) && res)
{
PQclear(res);
res = NULL;
***************
*** 906,912 ****
results = PQexec(pset.db, query);
/* these operations are included in the timing result: */
! OK = (AcceptResult(results, query) && ProcessCopyResult(results));
if (pset.timing)
GETTIMEOFDAY(&after);
--- 911,917 ----
results = PQexec(pset.db, query);
/* these operations are included in the timing result: */
! OK = (AcceptResult(results) && ProcessCopyResult(results));
if (pset.timing)
GETTIMEOFDAY(&after);
*** ./src/include/postgres_ext.h.orig Sat Nov 29 23:40:53 2003
--- ./src/include/postgres_ext.h Thu Mar 18 09:23:38 2004
***************
*** 58,63 ****
--- 58,64 ----
#define PG_DIAG_MESSAGE_PRIMARY 'M'
#define PG_DIAG_MESSAGE_DETAIL 'D'
#define PG_DIAG_MESSAGE_HINT 'H'
+ #define PG_DIAG_STATEMENT 'Q'
#define PG_DIAG_STATEMENT_POSITION 'P'
#define PG_DIAG_CONTEXT 'W'
#define PG_DIAG_SOURCE_FILE 'F'
*** ./src/include/utils/elog.h.orig Mon Mar 15 17:57:29 2004
--- ./src/include/utils/elog.h Thu Mar 18 09:35:38 2004
***************
*** 131,136 ****
--- 131,137 ----
extern int errfunction(const char *funcname);
extern int errposition(int cursorpos);
+ extern int errquery(const char *query);
/*----------
*** ./src/test/regress/output/create_function_1.source.orig Mon Mar 15 10:22:20
2004
--- ./src/test/regress/output/create_function_1.source Thu Mar 18 10:06:51 2004
***************
*** 57,62 ****
--- 57,64 ----
AS 'not even SQL';
ERROR: syntax error at or near "not" at character 1
CONTEXT: SQL function "test1"
+ LINE 1: not even SQL
+ ^
CREATE FUNCTION test1 (int) RETURNS int LANGUAGE sql
AS 'SELECT 1, 2, 3;';
ERROR: return type mismatch in function declared to return integer
---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
