Re: BufFileRead() error signalling

On 2020-Jun-08, Thomas Munro wrote:

> Stepping back a bit, one of the problems here is that we tried to
> model the functions on <stdio.h> fread(), but we didn't provide the
> companion ferror() and feof() functions, and then we were a bit fuzzy
> on when errno is set, even though the <stdio.h> functions don't
> document that. There were various ways forward, but the one that this
> patch follows is to switch to our regular error reporting system. The
> only thing that really costs us is marginally more vague error
> messages. Perhaps that could eventually be fixed by passing in some
> more context into calls like BufFileCreateTemp(), for use in error
> messages and perhaps also path names.

I think using our standard "exception" mechanism makes sense. As for
additional context, I think usefulness of the error messages would be
improved by showing the file path (because then user knows which
filesystem/tablespace was full, for example), but IMO any additional
context on top of that is of marginal additional benefit. If we really
cared, we could have errcontext() callbacks in the sites of interest,
but that would be a separate patch and perhaps not backpatchable.

> > + elog(ERROR, "could not seek block %ld temporary file", blknum);
> >
> > You mean "in temporary file" in the new message, no?
>
> Fixed.

The wording we use is "could not seek TO block N". (Or used to use,
before switching to pread/pwrite in most places, it seems).

Thanks!

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services