Re: fix "Success" error messages

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Shawn Debnath <sdn(at)amazon(dot)com>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: fix "Success" error messages
Date: 2019-06-19 02:51:55
Message-ID: 20190619025155.GD17450@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2019-06-19 02:54:26 Re: openssl valgrind failures on skink are due to openssl issue
Previous Message Andres Freund 2019-06-19 02:44:26 Re: openssl valgrind failures on skink are due to openssl issue