Re: BufFileRead() error signalling

On Fri, Jun 05, 2020 at 06:03:59PM +1200, Thomas Munro wrote:
> I didn't change BufFileWrite() to be void, to be friendly to existing
> callers outside the tree (if there are any), though I removed all the
> code that checks the return code. We can make it void later.

Missing one entry in AppendStringToManifest(). It sounds right to not
change the signature of the routine on back-branches to any ABI
breakages. It think that it could change on HEAD.

Anyway, why are we sure that it is fine to not complain even if
BufFileWrite() does a partial write? fwrite() is mentioned at the top
of the thread, but why is that OK?

> For the future: it feels a bit like we're missing a one line way to
> say "read this many bytes and error out if you run out".

- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not write block %ld of temporary file:
- %m",
- blknum)));
- }
+ elog(ERROR, "could not seek block %ld temporary file", blknum);

You mean "in temporary file" in the new message, no?

+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not write to \"%s\" : %m",
+ FilePathName(thisfile))));

Nit: "could not write [to] file \"%s\": %m" is a more common error
string.
--
Michael