On Tue, Jun 18, 2019 at 09:13:19AM -0700, Shawn Debnath wrote:
>>              case SLRU_WRITE_FAILED:
>>                      ereport(ERROR,
>>                                      (errcode_for_file_access(),
>>                                       errmsg("could not access status of 
>> transaction %u", xid),
>> -                                     errdetail("Could not write to file 
>> \"%s\" at offset %u: %m.",
>> -                                                       path, offset)));
>> +                                     errno == 0
>> +                                     ? errdetail("Short write to file 
>> \"%s\" at offset %u.", path, offset)
>> +                                     : errdetail("Could not write to file 
>> \"%s\" at offset %u: %m.",
>> +                                                             path, 
>> offset)));

There is no point to call errcode_for_file_access() if we know that
errno is 0.  Not a big deal, still..  The last time I looked at that,
the best way I could think of would be to replace slru_errcause with a
proper error string generated at error time.  Perhaps the partial
read/write case does not justify this extra facility.  I don't know.
At least that would be more flexible.

> Similarly, "Encountered partial write to file ..." would be better? Not 
> a 100% on using "Encountered" but "partial" seems to be the right word 
> to use here.

811b6e36 has done some work so as we have more consistency in the
error messages and I don't think we should introduce more flavors of
this stuff as that makes the life of translators harder.

-   if (CloseTransientFile(fd))
+   if (CloseTransientFile(fd) < 0)
Some spots are missing:
$ git grep "CloseTransientFile" | grep "))" | wc -l
30
--
Michael

Attachment: signature.asc
Description: PGP signature

Reply via email to