Improve error reporting in pg_upgrade's file copying/linking/rewriting.
The previous design for this had copyFile(), linkFile(), and
rewriteVisibilityMap() returning strerror strings, with the caller
producing one-size-fits-all error messages based on that. This made it
impossible to produce messages that described the failures with any degree
of precision, especially not short-read problems since those don't set
errno at all.
Since pg_upgrade has no intention of continuing after any error in this
area, let's fix this by just letting these functions call pg_fatal() for
themselves, making it easy for each point of failure to have a suitable
error message. Taking this approach also allows dropping cleanup code
that was unnecessary and was often rather sloppy about preserving errno.
To not lose relevant info that was reported before, pass in the schema name
and table name of the current table so that they can be included in the
error reports.
An additional problem was the use of getErrorText(), which was flat out
wrong for all but a couple of call sites, because it unconditionally did
"_dosmaperr(GetLastError())" on Windows. That's only appropriate when
reporting an error from a Windows-native API, which only a couple of
the callers were actually doing. Thus, even the reported strerror string
would be unrelated to the actual failure in many cases on Windows.
To fix, get rid of getErrorText() altogether, and just have call sites
do strerror(errno) instead, since that's the way all the rest of our
frontend programs do it. Add back the _dosmaperr() calls in the two
places where that's actually appropriate.
In passing, make assorted messages hew more closely to project style
guidelines, notably by removing initial capitals in not-complete-sentence
primary error messages. (I didn't make any effort to clean up places
I didn't have another reason to touch, though.)
Per discussion of a report from Thomas Kellerer. Back-patch to 9.6,
but no further; given the relative infrequency of reports of problems
here, it's not clear it's worth adapting the patch to older branches.
Patch by me, but with credit to Alvaro Herrera for spotting the issue
with getErrorText's misuse of _dosmaperr().
Discussion: <nsjrbh$8li$1@blaine.gmane.org>
Branch
------
master
Details
-------
http://git.postgresql.org/pg/commitdiff/f002ed2b8e45286fe73a36e119cba2016ea0de19
Modified Files
--------------
src/bin/pg_upgrade/check.c | 32 +++----
src/bin/pg_upgrade/controldata.c | 4 +-
src/bin/pg_upgrade/exec.c | 8 +-
src/bin/pg_upgrade/file.c | 182 +++++++++++++++++----------------------
src/bin/pg_upgrade/function.c | 6 +-
src/bin/pg_upgrade/option.c | 4 +-
src/bin/pg_upgrade/pg_upgrade.c | 2 +-
src/bin/pg_upgrade/pg_upgrade.h | 11 +--
src/bin/pg_upgrade/relfilenode.c | 42 ++++-----
src/bin/pg_upgrade/tablespace.c | 4 +-
src/bin/pg_upgrade/util.c | 15 ----
src/bin/pg_upgrade/version.c | 6 +-
12 files changed, 132 insertions(+), 184 deletions(-)