Re: Improving some plpgsql error messages

pá 20. 8. 2021 v 17:50 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:

> In commit 2f48ede08 I copied some error-message wording that had
> existed in exec_run_select() since 2003. I'm now dissatified
> with that, after realizing that it can produce output like this:
>
> ERROR: query "WITH a AS (
> SELECT
> regexp_split_to_table(info_out, '\n') AS info
> FROM
> public.results
> WHERE
> public.results.jid = id
> )
> SELECT
> * INTO tabular_info
> FROM
> a RETURN" is not a SELECT
> CONTEXT: PL/pgSQL function get_info(text) line 3 at RETURN QUERY
>
> There are a couple of things wrong with this:
>
> 1. The complaint is factually inaccurate, since the query obviously
> *is* a SELECT. It's the INTO part that's the problem, but good luck
> guessing that from the error text. (See [1] for motivation.)
>
> 2. It's fairly unreadable when the query is a long multi-line one,
> as it does a great job of burying the lede. This also violates
> our message style guideline that says primary error messages should
> be one-liners.
>
> The way to fix #1 is to provide a special case for SELECT INTO.
> In the attached I propose to fix #2 by moving the query text into
> an errcontext line, thus producing something like
>
> ERROR: query is SELECT INTO, but it should be plain SELECT
> CONTEXT: query: WITH a AS (
> SELECT
> regexp_split_to_table(info_out, '\n') AS info
> FROM
> public.results
> WHERE
> public.results.jid = id
> )
> SELECT
> * INTO tabular_info
> FROM
> a RETURN
> PL/pgSQL function get_info(text) line 3 at RETURN QUERY
>
> A case could also be made for just dropping the query text entirely,
> but I'm inclined to think that's not such a great idea. Certainly
> we ought to show the text in case of RETURN QUERY EXECUTE, where it
> won't be embedded in the function text.
>
> Looking around, I noted that exec_eval_expr() also has the bad
> habit of quoting the expression text right in the primary message,
> so the attached fixes that too. That case is slightly less bad
> since expressions are more likely to be short, but they surely
> aren't all short.
>
> Thoughts? Should I back-patch this into v14 where 2f48ede08
> came in, or just do it in HEAD?
>

It can be fixed in 14. This is a low risk patch.

Regards

Pavel

> regards, tom lane
>
> [1] https://www.postgresql.org/message-id/y65a6lco0z4.fsf%40mun.ca
>
>