Re: Mention column name in error messages

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>, Franck Verrot <franck(at)verrot(dot)fr>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Mention column name in error messages
Date: 2016-11-05 20:05:34
Message-ID: 21693.1478376334@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> ... I wonder whether we should
> not try to move in the direction of expanding the set of errors that we
> can provide error cursor info for. One aspect of that would be making
> sure that the text of the current statement is always available at
> runtime. I believe we do always have it somewhere now, but I'm not sure
> that it's passed through to the executor in any convenient way. The
> other aspect would then be to provide errlocation information based on
> the expression node that we're currently executing. That seems do-able
> probably. The overhead would likely consist of storing a pointer to the
> current node someplace where an error context callback could find it.
> Which is not zero cost, but I think it wouldn't be awful.

Just to demonstrate what I'm talking about, I made a really dirty
proof-of-concept patch, attached. With this, you get results like

regression=# create table foo(bar varchar(4), baz varchar(2));
CREATE TABLE
regression=# insert into foo values ('foo2!', 'ok');
ERROR: value too long for type character varying(4)
LINE 1: insert into foo values ('foo2!', 'ok');
^

which was the original complaint, and it also works for run-time cases:

regression=# insert into foo values (random()::text, 'ok');
ERROR: value too long for type character varying(4)
LINE 1: insert into foo values (random()::text, 'ok');
^

If that seems like a satisfactory behavior, somebody could push forward
with turning this into a committable patch. There's a lot to do:

* I just used debug_query_string as the statement text source, which means
it only works correctly for errors in directly client-issued statements.
We'd need some work on appropriately passing the correct text down to
execution. (It would be a good idea to decouple elog.c from
debug_query_string while at it, I think.)

* I didn't do anything about nested execution contexts. You'd want only
the innermost one to set the cursor pointer, but as the code stands
I think the outermost one would win.

* Some attention needs to be paid to factorizing the support nicely
--- multiple copies of the callback isn't good, and we might end up
with many copies of the setup boilerplate. Maybe the parser's
pcb_error_callback infrastructure would be a useful prototype.

* I only bothered to hook in execution of function/operator nodes.
That's probably a 90% solution already, but it's not meant to be
the final version.

Likely, doing anything about the last point should wait on Andres' work
with linearizing expression execution. As this patch stands, you have to
think carefully about where within an ExecEvalFoo function is the right
place to set cur_expr, since it shouldn't get set till after control
returns from subsidiary nodes. But those recursive calls would go away.

regards, tom lane

Attachment Content-Type Size
cursor-positions-for-runtime-errors-crude-hack.patch text/x-diff 9.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2016-11-05 20:39:47 Re: Add support for SRF and returning composites to pl/tcl
Previous Message Andrew Dunstan 2016-11-05 19:11:22 Re: btree_gin and btree_gist for enums