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