pgsql: Rework error messages around file handling

Started by Michael Paquierabout 8 years ago8 messagescomitters
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

won't retrysuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t209550
psql -h localhost -U postgres

Built from patchset v3 (message #3), July 29, 2026 at 12:56 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t209550_3 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t209550_3 && git checkout t209550_3

Patchset v3 (message #3) is on t209550_3

Jump to latest
#1Michael Paquier
michael@paquier.xyz

Rework error messages around file handling

Some error messages related to file handling are using the code path
context to define their state. For example, 2PC-related errors are
referring to "two-phase status files", or "relation mapping file" is
used for catalog-to-filenode mapping, however those prove to be
difficult to translate, and are not more helpful than just referring to
the path of the file being worked on. So simplify all those error
messages by just referring to files with their path used. In some
cases, like the manipulation of WAL segments, the context is actually
helpful so those are kept.

Calls to the system function read() have also been rather inconsistent
with their error handling sometimes not reporting the number of bytes
read, and some other code paths trying to use an errno which has not
been set. The in-core functions are using a more consistent pattern
with this patch, which checks for both errno if set or if an
inconsistent read is happening.

So as to care about pluralization when reading an unexpected number of
byte(s), "could not read: read %d of %zu" is used as error message, with
%d field being the output result of read() and %zu the expected size.
This simplifies the work of translators with less variations of the same
message.

Author: Michael Paquier
Reviewed-by: Álvaro Herrera
Discussion: /messages/by-id/20180520000522.GB1603@paquier.xyz

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/811b6e36a9e21a4d9eb78410976c88ce601cea0c

Modified Files
--------------
src/backend/access/transam/twophase.c | 40 +++++++++--------
src/backend/access/transam/xlog.c | 40 +++++++++++------
src/backend/replication/logical/origin.c | 13 ++++--
src/backend/replication/logical/snapbuild.c | 68 ++++++++++++++++++++---------
src/backend/replication/slot.c | 26 +++++++----
src/backend/replication/walsender.c | 20 +++++++--
src/backend/utils/cache/relmapper.c | 28 +++++++-----
src/bin/pg_basebackup/pg_receivewal.c | 12 +++--
src/bin/pg_rewind/file_ops.c | 14 ++++--
src/bin/pg_rewind/parsexlog.c | 14 ++++--
src/bin/pg_waldump/pg_waldump.c | 19 +++++---
src/common/controldata_utils.c | 8 ++--
12 files changed, 207 insertions(+), 95 deletions(-)

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#1)
Re: pgsql: Rework error messages around file handling

Michael Paquier <michael@paquier.xyz> writes:

Rework error messages around file handling

longfin thinks you missed at least one cast ...

regards, tom lane

#3Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#2)
Re: pgsql: Rework error messages around file handling

On Tue, Jul 17, 2018 at 07:23:04PM -0400, Tom Lane wrote:

Michael Paquier <michael@paquier.xyz> writes:

Rework error messages around file handling

longfin thinks you missed at least one cast ...

Hm, I checked that. Just casting to (Size) should do it. I cannot get
a warning with neither clang nor gcc on Debian SID even with -Wformat,
and no OSX at hand.

So the attached would fix the problem?
--
Michael

Attachments:

t209550_3
printf-zu-osx.patchtext/x-diff; charset=us-asciiDownload+1-1
#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#3)
Re: pgsql: Rework error messages around file handling

Michael Paquier <michael@paquier.xyz> writes:

On Tue, Jul 17, 2018 at 07:23:04PM -0400, Tom Lane wrote:

longfin thinks you missed at least one cast ...

Hm, I checked that. Just casting to (Size) should do it.

Sounds right to me.

regards, tom lane

#5Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#4)
Re: pgsql: Rework error messages around file handling

On Tue, Jul 17, 2018 at 08:02:43PM -0400, Tom Lane wrote:

Sounds right to me.

Thanks for double-checking, pushed.
--
Michael

#6Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#5)
Re: pgsql: Rework error messages around file handling

On Wed, Jul 18, 2018 at 09:14:14AM +0900, Michael Paquier wrote:

Thanks for double-checking, pushed.

This attempt made longfin happier for the path patched, but there were a
couple of other ones which were hidden behind. I got them fixed now,
which should bring the machine back to green.
--
Michael

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#6)
Re: pgsql: Rework error messages around file handling

Michael Paquier <michael@paquier.xyz> writes:

This attempt made longfin happier for the path patched, but there were a
couple of other ones which were hidden behind. I got them fixed now,
which should bring the machine back to green.

Hm, I'd just tried this on my Mac laptop and saw only the one in
walsender.c. No objection to the other two though, they might be
relevant on other platforms.

regards, tom lane

#8Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#7)
Re: pgsql: Rework error messages around file handling

On Tue, Jul 17, 2018 at 09:13:28PM -0400, Tom Lane wrote:

Hm, I'd just tried this on my Mac laptop and saw only the one in
walsender.c. No objection to the other two though, they might be
relevant on other platforms.

longfin has turned back to green again, so at least this is fixed now.
--
Michael