On Mon, Jan 27, 2020 at 10:09:30AM -0500, Robert Haas wrote:
> I recognize that your commit message explains this change by saying
> that this code will now never be reached except as a result of a short
> read, but I don't think that will necessarily be clear to future
> readers of those code, or people who get the error message. It seems
> like if we're going to do do this, the error messages ought to be
> changed to complain about a short read rather than an inability to
> read for unspecified reasons. However, I wonder why we don't make
> BufFileRead() throw all of the errors including complaining about
> short reads. I would like to confess my undying (and probably
> unrequited) love for the following code from md.c:
>
> errmsg("could not read block
> %u in file \"%s\": read only %d of %d bytes",
>
> Now that is an error message! I am not confused! I don't know why that
> happened, but I sure know what happened!I was briefly looking at 0001, and count -1 from me for the formulation of the error messages used in those patches. > I think we should aim for that kind of style everywhere, so that > complaints about reading and writing files are typically reported as > either of these: > > could not read file "%s": %m > could not read file "%s": read only %d of %d bytes That's actually not the best fit, because this does not take care of the pluralization of the second message if you have only 1 byte to read ;) A second point to take into account is that the unification of error messages makes for less translation work, which is always welcome. Those points have been discussed on this thread: https://www.postgresql.org/message-id/20180520000522.gb1...@paquier.xyz The related commit is 811b6e3, and the pattern we agreed on for a partial read was: "could not read file \"%s\": read %d of %zu" Then, if the syscall had an error we'd fall down to that: "could not read file \"%s\": %m" -- Michael
signature.asc
Description: PGP signature
