Fix oversight in pts_error_callback()
Since we will invoke callback functions unconditionally in errfinish(), so
pts_error_callback() should not report "invalid type name" without checking
current error status.
Regards,
Qingqing
Index: src/backend/parser/parse_type.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/parse_type.c,v
retrieving revision 1.76
diff -c -r1.76 parse_type.c
*** src/backend/parser/parse_type.c 1 Aug 2005 20:31:10 -0000 1.76
--- src/backend/parser/parse_type.c 9 Aug 2005 05:53:08 -0000
***************
*** 359,373 ****
pts_error_callback(void *arg)
{
const char *str = (const char *) arg;
!
! errcontext("invalid type name \"%s\"", str); /*
* Currently we just suppress any syntax error position report,
rather
* than transforming to an "internal query" error. It's
unlikely that
* a type name is complex enough to need positioning.
*/
! errposition(0);
}
/*
--- 359,379 ----
pts_error_callback(void *arg)
{
const char *str = (const char *) arg;
! int syntaxerrposition; /*
* Currently we just suppress any syntax error position report,
rather
* than transforming to an "internal query" error. It's
unlikely that
* a type name is complex enough to need positioning.
*/
! syntaxerrposition = geterrposition();
! if (syntaxerrposition > 0)
! {
! errcontext("invalid type name \"%s\"", str);
! errposition(0);
! }
! else
! errcontext("parse type string \"%s\"", str);
}
/*
"Qingqing Zhou" <zhouqq@cs.toronto.edu> writes:
Since we will invoke callback functions unconditionally in errfinish(), so
pts_error_callback() should not report "invalid type name" without checking
current error status.
Please exhibit a case in which you feel this is needed.
regards, tom lane
"Tom Lane" <tgl@sss.pgh.pa.us> writes
Please exhibit a case in which you feel this is needed.
Suppose I want to print a debug info in parseTypeString() like this:
+ elog(DEBUG1, "parse type %s", buf.data);
raw_parsetree_list = raw_parser(buf.data);
Rebuild the server, psql it:
test=# set log_min_messages = debug1;
SET
test=# select regtypein('int4');
DEBUG: parse type SELECT NULL::int4
CONTEXT: invalid type name "int4"
regtypein
-----------
integer
(1 row)
The CONTEXT info is bogus.
"Qingqing Zhou" <zhouqq@cs.toronto.edu> writes:
"Tom Lane" <tgl@sss.pgh.pa.us> writes
Please exhibit a case in which you feel this is needed.
Suppose I want to print a debug info in parseTypeString() like this:
+ elog(DEBUG1, "parse type %s", buf.data);
raw_parsetree_list = raw_parser(buf.data);
That's a contrived example (and if I believed it, I would think that the
right answer is to emit no errcontext if the elevel is less than ERROR).
Give me an actual use case in which the patch gives a better rather than
worse error report. I think for most people it would just obfuscate the
message.
regards, tom lane
"Tom Lane" <tgl@sss.pgh.pa.us> writes:
That's a contrived example (and if I believed it, I would think that the
right answer is to emit no errcontext if the elevel is less than ERROR).
Yes, I've thought about ignore errcontext by considering elevel. But I
scratched the source code for other uses of errcontext, and I found seems
all of them understand that errcontext will be called unconditionally. For
example, buffer_write_error_callback(), it doesn't say "error in writing
block ..." but says "writing block ...". So I think this place is not
consistent with others - it just says "invalid ... ", and should be changed.
Regards,
Qingqing