> On 6 Dec 2023, at 14:03, Nazir Bilal Yavuz <byavu...@gmail.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



Reply via email to