Re: Add missing error codes to PANIC/FATAL error reports in xlog.c and relcache.c

> On 6 Dec 2023, at 14:03, Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com> wrote:

> There is an ongoing thread [1] for adding missing SQL error codes to
> PANIC and FATAL error reports in xlogrecovery.c file. I did the same
> but for xlog.c and relcache.c files.

- elog(PANIC, "space reserved for WAL record does not match what was written");
+ ereport(PANIC,
+ (errcode(ERRCODE_DATA_CORRUPTED),
+ errmsg("space reserved for WAL record does not match what was written")));

elogs turned into ereports should use errmsg_internal() to keep the strings
from being translated.

- elog(FATAL, "could not write init file");
+ ereport(FATAL,
+ (errcode_for_file_access(),
+ errmsg("could not write init file")));

Is it worthwhile adding %m on these to get a little more help when debugging
errors that shouldn't happen?

- elog(FATAL, "could not write init file");
+ ereport(FATAL,
+ (errcode_for_file_access(),

The extra parenthesis are no longer needed, I don't know if we have a policy to
remove them when changing an ereport call but we should at least not introduce
new ones.

- elog(FATAL, "cannot read pg_class without having selected a database");
+ ereport(FATAL,
+ (errcode(ERRCODE_INTERNAL_ERROR),

ereport (and thus elog) already defaults to ERRCODE_INTERNAL_ERROR for ERROR or
higher, so unless there is a better errcode an elog() call if preferrable here.

> I couldn't find a suitable error code for the "cache lookup failed for
> relation" error in relcache.c and this error comes up in many places.
> Would it be reasonable to create a new error code specifically for
> this?

We use ERRCODE_UNDEFINED_OBJECT for similar errors elsewhere, perhaps we can
use that for these as well?

--
Daniel Gustafsson