Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 1a990b2

Browse files
Have BufFileSize() ereport() on FileSize() failure.
Move the responsibility for checking for and reporting a failure from the only current BufFileSize() caller, logtape.c, to BufFileSize() itself. Code within buffile.c is generally responsible for interfacing with fd.c to report irrecoverable failures. This seems like a convention that's worth sticking to. Reorganizing things this way makes it easy to make the error message raised in the event of BufFileSize() failure descriptive of the underlying problem. We're now clear on the distinction between temporary file name and BufFile name, and can show errno, confident that its value actually relates to the error being reported. In passing, an existing, similar buffile.c ereport() + errcode_for_file_access() site is changed to follow the same conventions. The API of the function BufFileSize() is changed by this commit, despite already being in a stable release (Postgres 11). This seems acceptable, since the BufFileSize() ABI was changed by commit aa55183, which hasn't made it into a point release yet. Besides, it's difficult to imagine a third party BufFileSize() caller not just raising an error anyway, since BufFile state should be considered corrupt when BufFileSize() fails. Per complaint from Tom Lane. Discussion: https://postgr.es/m/26974.1540826748@sss.pgh.pa.us Backpatch: 11-, where shared BufFiles were introduced.
1 parent f2cbffc commit 1a990b2

File tree

2 files changed

+11
-8
lines changed

2 files changed

+11
-8
lines changed

src/backend/storage/file/buffile.c

+11-4
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,8 @@ BufFileOpenShared(SharedFileSet *fileset, const char *name)
304304
if (nfiles == 0)
305305
ereport(ERROR,
306306
(errcode_for_file_access(),
307-
errmsg("could not open BufFile \"%s\"", name)));
307+
errmsg("could not open temporary file \"%s\" from BufFile \"%s\": %m",
308+
segment_name, name)));
308309

309310
file = makeBufFileCommon(nfiles);
310311
file->files = files;
@@ -763,20 +764,26 @@ BufFileTellBlock(BufFile *file)
763764
#endif
764765

765766
/*
766-
* Return the current file size.
767+
* Return the current shared BufFile size.
767768
*
768769
* Counts any holes left behind by BufFileAppend as part of the size.
769-
* Returns -1 on error.
770+
* ereport()s on failure.
770771
*/
771772
int64
772773
BufFileSize(BufFile *file)
773774
{
774775
int64 lastFileSize;
775776

777+
Assert(file->fileset != NULL);
778+
776779
/* Get the size of the last physical file. */
777780
lastFileSize = FileSize(file->files[file->numFiles - 1]);
778781
if (lastFileSize < 0)
779-
return -1;
782+
ereport(ERROR,
783+
(errcode_for_file_access(),
784+
errmsg("could not determine size of temporary file \"%s\" from BufFile \"%s\": %m",
785+
FilePathName(file->files[file->numFiles - 1]),
786+
file->name)));
780787

781788
return ((file->numFiles - 1) * (int64) MAX_PHYSICAL_FILESIZE) +
782789
lastFileSize;

src/backend/utils/sort/logtape.c

-4
Original file line numberDiff line numberDiff line change
@@ -433,10 +433,6 @@ ltsConcatWorkerTapes(LogicalTapeSet *lts, TapeShare *shared,
433433
pg_itoa(i, filename);
434434
file = BufFileOpenShared(fileset, filename);
435435
filesize = BufFileSize(file);
436-
if (filesize < 0)
437-
ereport(ERROR,
438-
(errcode_for_file_access(),
439-
errmsg("could not determine size of temporary file \"%s\"", filename)));
440436

441437
/*
442438
* Stash first BufFile, and concatenate subsequent BufFiles to that.

0 commit comments

Comments
 (0)