Thread: Re: [PATCHES] Proposed patch for error locations
[ cross-posting to pgsql-interfaces in hopes of drawing more comments ] Michael Glaesemann <grzm@myrealbox.com> writes: > On Mar 13, 2006, at 2:37 , Tom Lane wrote: >> http://archives.postgresql.org/pgsql-patches/2006-03/msg00153.php > This looks really nice. >> One thing I'm noticing already is that the addition of "at character N" >> to a lot of these messages isn't an improvement. In psql it's >> certainly redundant with the error-cursor display. > The pure character count is definitely difficult to use with larger > queries. I think it could be more useful if it were > line:char_of_line. Would others find this useful? The change in behavior would actually be in libpq, because it's PQerrorMessage that is doing the deed (assuming a reasonably modern server and libpq). What I was considering proposing was that we migrate the error-cursor feature out of psql and into PQerrorMessage. This would mean that you'd get responses like ERROR: column "foo" does not exist LINE 1: select foo from a; ^ from all libpq-using applications not just psql. We could make this conditional on the error verbosity --- in "terse" mode the "LINE N" output wouldn't appear, and "at character N" still would. Applications should already be expecting multiline outputs from PQerrorMessage if they're in non-terse mode, so this ought to be OK. Comments? regards, tom lane
Tom Lane wrote: > The change in behavior would actually be in libpq, because it's > PQerrorMessage that is doing the deed (assuming a reasonably modern > server and libpq). What I was considering proposing was that we migrate > the error-cursor feature out of psql and into PQerrorMessage. This > would mean that you'd get responses like > > ERROR: column "foo" does not exist > LINE 1: select foo from a; > ^ This doesn't work on terminal using a variable-width font, does it? Sure, you can have all the interfaces use a monospace font, but it seems a weird thing to do. I think the line/character position should be returned in a separate error attribute in ereport. So for example pgAdmin could count characters and mark it in bold or use a different color. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera <alvherre@commandprompt.com> writes:
> This doesn't work on terminal using a variable-width font, does it?
> Sure, you can have all the interfaces use a monospace font, but it seems
> a weird thing to do. I think the line/character position should be
> returned in a separate error attribute in ereport. So for example
> pgAdmin could count characters and mark it in bold or use a different
> color.
That information is already available to pgAdmin, and has been since
7.4; if they are failing to exploit it that's their problem not libpq's.
Basically what's at stake here is the behavior of "dumb" applications
that are just using PQerrorMessage and not doing anything smart with
error message fields. It does not seem unreasonable to me to assume
fixed-width font in that context. We haven't seen any complaints about
psql doing it have we?
regards, tom lane
Tom Lane wrote: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > This doesn't work on terminal using a variable-width font, does it? > > Sure, you can have all the interfaces use a monospace font, but it seems > > a weird thing to do. I think the line/character position should be > > returned in a separate error attribute in ereport. So for example > > pgAdmin could count characters and mark it in bold or use a different > > color. > > That information is already available to pgAdmin, and has been since > 7.4; if they are failing to exploit it that's their problem not libpq's. Aye, that's fine. I don't really know if pgAdmin uses the info or not. I thought the proposal to remove the "at character N" in the error message meant that the only way to get that information would be from the "LINE Y: ...\n[some whitespace]^" message, which would be pretty cumbersome. But if there already is a field in the error message for the position, then nothing is lost. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Tom Lane wrote:
>> That information is already available to pgAdmin, and has been since
>> 7.4; if they are failing to exploit it that's their problem not libpq's.
> Aye, that's fine. I don't really know if pgAdmin uses the info or not.
> I thought the proposal to remove the "at character N" in the error
> message meant that the only way to get that information would be from
> the "LINE Y: ...\n[some whitespace]^" message, which would be pretty
> cumbersome.
You should reflect on the fact that psql is currently generating that
line from outside libpq ...
regards, tom lane
I took another look at this and realized that for PQerrorMessage to emit
a cursor-pointer line, it'd be necessary for libpq to hold onto a copy
of the query last sent, which it doesn't do presently. This is annoying
for long queries --- in the worst case it could provoke out-of-memory
failures that don't occur now.
Plan B would be for psql to stop using PQerrorMessage and generate the
message report text from the message fields, omitting "at character N".
This would require duplicating a couple dozen lines from inside libpq.
It'd also mean that the report text gets built twice, once inside libpq
and once by psql, which is probably not such a big deal since error
messages ought not be a performance-critical path ... except there's
still that little question of overrunning memory.
Plan C is just to drop the "at character N" string from what
PQerrorMessage generates. We could make this configurable (maybe via
additional PGVerbosity values), or just not worry about whether it's
important.
Thoughts?
regards, tom lane
> from all libpq-using applications not just psql. We could make this > conditional on the error verbosity --- in "terse" mode the "LINE N" > output wouldn't appear, and "at character N" still would. Applications > should already be expecting multiline outputs from PQerrorMessage if > they're in non-terse mode, so this ought to be OK. Comments? Sounds like it'd be handy in phpPgAdmin...