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