Re: Better error reporting from extension scripts (Was: Extend ALTER OPERATOR)

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Christoph Berg <myon(at)debian(dot)org>
Cc: Michael Banck <mbanck(at)gmx(dot)net>, Tommy Pavlicek <tommypav122(at)gmail(dot)com>, Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org>, pgsql-hackers(at)lists(dot)postgresql(dot)org, jian(dot)universality(at)gmail(dot)com
Subject: Re: Better error reporting from extension scripts (Was: Extend ALTER OPERATOR)
Date: 2024-09-28 19:45:36
Message-ID: 1092841.1727552736@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> It looks like if we did want to suppress that, the right fix is to
> make gram.y track statement start locations more honestly, as in
> 0002 attached (0001 is the same as before).

I did a little bit of further work on this:

* I ran some performance checks and convinced myself that indeed
the more complex definition of YYLLOC_DEFAULT has no measurable
cost compared to the overall runtime of raw_parser(), never mind
later processing. So I see no reason not to go ahead with that
change. I swapped the two patches to make that 0001, and added
a regression test illustrating its effect on pg_stat_statements.
(Without the gram.y change, the added slash-star comment shows
up in the pg_stat_statements output, which is surely odd.)

* I concluded that the best way to report the individual statement
when we're able to do that is to include it in an errcontext()
message, similar to what spi.c's _SPI_error_callback does.
Otherwise it interacts badly if some more-tightly-nested error
context function has already set up an "internal error query",
as for example SQL function errors will do if you enable
check_function_bodies = on.

So here's v3, now with commit messages and regression tests.
I feel like this is out of the proof-of-concept stage and
might now actually be committable. There's still a question
of whether reporting the whole script as the query is OK when
we have a syntax error, but I have no good ideas as to how to
make that terser. And I think you're right that we shouldn't let
perfection stand in the way of making things noticeably better.

regards, tom lane

Attachment Content-Type Size
v3-0001-Improve-parser-s-reporting-of-statement-start-loc.patch text/x-diff 9.1 KB
v3-0002-Improve-reporting-of-errors-in-extension-script-f.patch text/x-diff 17.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2024-09-28 19:49:56 Re: msys inet_pton strangeness
Previous Message Andrew Dunstan 2024-09-28 17:26:21 Re: msys inet_pton strangeness