Better error messages for %TYPE and %ROWTYPE in plpgsql

po 26. 2. 2024 v 21:02 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Per recent discussion[1], plpgsql returns fairly unhelpful "syntax
error" messages when a %TYPE or %ROWTYPE construct references a
nonexistent object. Here's a quick little finger exercise to try
to improve that.

The basic point is that plpgsql_parse_wordtype and friends are
designed to return NULL rather than failing (at least when it's
easy to do so), but that leaves the caller without enough info
to deliver a good error message. There is only one caller,
and it has no use at all for this behavior, so let's just
change those functions to throw appropriate errors. Amusingly,
plpgsql_parse_wordrowtype was already behaving that way, and
plpgsql_parse_cwordrowtype did so in more cases than not,
so we didn't even have a consistent "return NULL" story.

Along the way I got rid of plpgsql_parse_cwordtype's restriction
on what relkinds can be referenced. I don't really see the
point of that --- as long as the relation has the desired
column, the column's type is surely well-defined.

+1

Pavel

Per recent discussion[1], plpgsql returns fairly unhelpful "syntax
error" messages when a %TYPE or %ROWTYPE construct references a
nonexistent object. Here's a quick little finger exercise to try
to improve that.

Looks this modify the error message, I want to know how ould we treat
error-message-compatible issue during minor / major upgrade. I'm not
sure if my question is too inconceivable, I ask this because one of my
patch [1]/messages/by-id/87r0hmvuvr.fsf@163.com has blocked on this kind of issue [only] for 2 months and
actaully the error-message-compatible requirement was metioned by me at
the first and resolve it by adding a odd parameter. Then the odd
parameter blocked the whole process.

[1]: /messages/by-id/87r0hmvuvr.fsf@163.com

--
Best Regards
Andy Fan

On Mon, Feb 26, 2024 at 5:46 PM Andy Fan <zhihuifan1213@163.com> wrote:

Per recent discussion[1], plpgsql returns fairly unhelpful "syntax
error" messages when a %TYPE or %ROWTYPE construct references a
nonexistent object. Here's a quick little finger exercise to try
to improve that.

Looks this modify the error message, I want to know how ould we treat
error-message-compatible issue during minor / major upgrade.

There is no bug here so no back-patch; and we are not yet past feature
freeze for v17.

David J.

"David G. Johnston" <david.g.johnston@gmail.com> writes:

On Mon, Feb 26, 2024 at 5:46 PM Andy Fan <zhihuifan1213@163.com> wrote:

Looks this modify the error message,

Well, yeah, that's sort of the point.

I want to know how ould we treat
error-message-compatible issue during minor / major upgrade.

There is no bug here so no back-patch; and we are not yet past feature
freeze for v17.

Indeed, I did not intend this for back-patch. However, I'm having
a hard time seeing the errors in question as something you'd have
automated handling for, so I don't grasp why you would think
there's a compatibility hazard.

regards, tom lane

"David G. Johnston" <david.g.johnston@gmail.com> writes:

On Mon, Feb 26, 2024 at 5:46 PM Andy Fan <zhihuifan1213@163.com> wrote:

Per recent discussion[1], plpgsql returns fairly unhelpful "syntax
error" messages when a %TYPE or %ROWTYPE construct references a
nonexistent object. Here's a quick little finger exercise to try
to improve that.

Looks this modify the error message, I want to know how ould we treat
error-message-compatible issue during minor / major upgrade.

There is no bug here so no back-patch; and we are not yet past feature freeze for v17.

Acutally I didn't asked about back-patch. I meant error message is an
part of user interface, if we change a error message, the end
user may be impacted, at least in theory. for example, end-user has some
code like this:

String errMsg = ex.getErrorMsg();

if (errMsg.includes("a-target-string"))
{
// do sth.
}

So if the error message is changed, the above code may be broken.

I have little experience on this, so I want to know the policy we are
using, for the background which I said in previous reply.

--
Best Regards
Andy Fan

On Mon, Feb 26, 2024 at 6:54 PM Andy Fan <zhihuifan1213@163.com> wrote:

"David G. Johnston" <david.g.johnston@gmail.com> writes:

On Mon, Feb 26, 2024 at 5:46 PM Andy Fan <zhihuifan1213@163.com> wrote:

Per recent discussion[1], plpgsql returns fairly unhelpful "syntax
error" messages when a %TYPE or %ROWTYPE construct references a
nonexistent object. Here's a quick little finger exercise to try
to improve that.

Looks this modify the error message, I want to know how ould we treat
error-message-compatible issue during minor / major upgrade.

There is no bug here so no back-patch; and we are not yet past feature

freeze for v17.

Acutally I didn't asked about back-patch.

What else should I be understanding when you write the words "minor
upgrade"?

So if the error message is changed, the above code may be broken.

A fair point to bring up, and is change-specific. User-facing error
messages should be informative and where they are not changing them is
reasonable. Runtime errors probably need more restraint since they are
more likely to be in a production monitoring alerting system but anything
that is reporting what amounts to a syntax error should be reasonable to
change and not expect people to be writing production code looking for
them. This seems to fall firmly into the "badly written code"/syntax
category.

David J.