Horton

Thread

Commits

  1. Improve plpgsql's error messages for incorrect %TYPE and %ROWTYPE.

  1. Better error messages for %TYPE and %ROWTYPE in plpgsql

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-02-26T20:02:20Z

    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.

			regards, tom lane

[1] https://www.postgresql.org/message-id/flat/88b574f4-cc08-46c5-826b-020849e5a356%40gelassene-pferde.biz

  2. Re: Better error messages for %TYPE and %ROWTYPE in plpgsql

    Pavel Stehule <pavel.stehule@gmail.com> — 2024-02-26T20:10:08Z

    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


>                         regards, tom lane
>
> [1]
> https://www.postgresql.org/message-id/flat/88b574f4-cc08-46c5-826b-020849e5a356%40gelassene-pferde.biz
>
>

  3. Re: Better error messages for %TYPE and %ROWTYPE in plpgsql

    Andy Fan <zhihuifan1213@163.com> — 2024-02-27T00:40:17Z

    
> 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] 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] https://www.postgresql.org/message-id/87r0hmvuvr.fsf@163.com

-- 
Best Regards
Andy Fan

  4. Re: Better error messages for %TYPE and %ROWTYPE in plpgsql

    David G. Johnston <david.g.johnston@gmail.com> — 2024-02-27T00:54:05Z

    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.

  5. Re: Better error messages for %TYPE and %ROWTYPE in plpgsql

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-02-27T01:27:24Z

    "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

  6. Re: Better error messages for %TYPE and %ROWTYPE in plpgsql

    Andy Fan <zhihuifan1213@163.com> — 2024-02-27T01:49:00Z

    "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

  7. Re: Better error messages for %TYPE and %ROWTYPE in plpgsql

    David G. Johnston <david.g.johnston@gmail.com> — 2024-02-27T02:01:55Z

    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.