Re: Missing errcode() in ereport - Mailing list pgsql-hackers

Hi,

On 2020-03-19 14:07:04 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2020-03-17 10:09:18 -0400, Tom Lane wrote:
> >> We might want to spend some effort thinking how to find or prevent
> >> additional bugs of the same ilk ...
> 
> > Yea, that'd be good.  Trying to help people new to postgres write their
> > first patches I found that ereport is very confusing to them - largely
> > because the syntax doesn't make much sense. Made worse by the compiler
> > error messages being terrible in many cases.
> 
> > Not sure there's much we can do without changing ereport's "signature"
> > though :(
> 
> Now that we can rely on having varargs macros, I think we could
> stop requiring the extra level of parentheses, ie instead of
> 
>         ereport(ERROR,
>                 (errcode(ERRCODE_DIVISION_BY_ZERO),
>                  errmsg("division by zero")));
> 
> it could be just
> 
>         ereport(ERROR,
>                 errcode(ERRCODE_DIVISION_BY_ZERO),
>                 errmsg("division by zero"));
> 
> (The old syntax had better still work, of course.  I'm not advocating
> running around and changing existing calls.)

I think that'd be an improvement, because:

> I'm not sure that this'd really move the goalposts much in terms of making
> it any less confusing, but possibly it would improve the compiler errors?
> Do you have any concrete examples of crummy error messages?

ane of the ones I saw confuse people is just:
/home/andres/src/postgresql/src/backend/tcop/postgres.c:3727:4: error: ‘ereport’ undeclared (first use in this
function);did you mean ‘rresvport’?
 
 3727 |    ereport(FATAL,
      |    ^~~~~~~
      |    rresvport
/home/andres/src/postgresql/src/backend/tcop/postgres.c:3727:4: note: each undeclared identifier is reported only once
foreach function it appears in
 

because the extra parens haven't been added.


I personally actually hit a variant of that on a semi-regular basis:
Closing the parens for "rest" early, as there's just so many parens in
ereports, especially if an errmsg argument is a function call
itself. Which leads to a variation of the above error message.  I know
how to address it, obviously, but I do find it somewhat annoying to deal
with.

Another one I've both seen and committed byself is converting an elog to
an ereport, and not adding an errcode() around the error code - which
silently looks like it works.

Greetings,

Andres Freund