Re: More consistency for some file-related error message

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Postgres hackers <pgsql-hackers@postgresql.org>
Date: 2018-07-19T03:14:36Z
Lists: pgsql-hackers

Attachments

On Wed, Jul 18, 2018 at 10:57:16AM -0400, Tom Lane wrote:
> +1.

Okay, thanks.  I can always get that pushed first if there are no
objections.  More can be always done, but that's already a nice cut.

> The short-read ereport has no errcode() call, meaning it will report
> XX000, which seems like it's against project policy for foreseeable
> errors.  In this example ERRCODE_DATA_CORRUPTED seems better.

Okay, my mistake then.  All the error code paths for read really out to
not fail, so ERRCODE_DATA_CORRUPTED seems adapted for all of them to
me instead of ERRCODE_INTERNAL_ERROR.  Or would we want to create new
sub categories for corruptions?  I can personally live with
ERRCODE_DATA_CORRUPTED but..

By the way, the original PANIC message in StartupReplicationOrigin also
lacked an errcode, so fixed on the way.

> BTW, isn't the initial "errno = 0" dead code now?

Hm.  I have not bothered touching those as it could be possible that
read() may not initialize errno to 0, so errno would remain set to any
previous value when less bytes than expected are read, no?  It seems to
me that the current coding is more careful.

Attached are two patches:
- 0001 is the previous patch with a commit messages.
- 0002 is an answer to Tom's remark about errcodes in the new error
messages for read().  I have left out the error messages for 2PC, which
are WARNINGs (those should actually be ERROR, promoted to FATAL in the
startup process, see here: https://commitfest.postgresql.org/19/1717/).
--
Michael

Commits

  1. Add proper errcodes to new error messages for read() failures

  2. Make more consistent some error messages for file-related operations