Skip to content

Commit 95647a1

Browse files
macdicemacdice
committed
Fix buffile.c error handling.
Convert buffile.c error handling to use ereport. This fixes cases where I/O errors were indistinguishable from EOF or not reported. Also remove "%m" from error messages where errno would be bogus. While we're modifying those strings, add block numbers and short read byte counts where appropriate. Back-patch to all supported releases. Reported-by: Amit Khandekar <amitdkhan.pg@gmail.com> Reviewed-by: Melanie Plageman <melanieplageman@gmail.com> Reviewed-by: Alvaro Herrera <alvherre@2ndquadrant.com> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Reviewed-by: Ibrar Ahmed <ibrar.ahmad@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/CA%2BhUKGJE04G%3D8TLK0DLypT_27D9dR8F1RQgNp0jK6qR0tZGWOw%40mail.gmail.com
1 parent 9170da9 commit 95647a1

File tree

5 files changed

+86
-94
lines changed

5 files changed

+86
-94
lines changed

‎src/backend/access/gist/gistbuildbuffers.c

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -757,26 +757,20 @@ gistRelocateBuildBuffersOnSplit(GISTBuildBuffers *gfbb, GISTSTATE *giststate,
757757
static void
758758
ReadTempFileBlock(BufFile *file, long blknum, void *ptr)
759759
{
760+
size_t nread;
761+
760762
if (BufFileSeekBlock(file, blknum) != 0)
761-
elog(ERROR, "could not seek temporary file: %m");
762-
if (BufFileRead(file, ptr, BLCKSZ) != BLCKSZ)
763-
elog(ERROR, "could not read temporary file: %m");
763+
elog(ERROR, "could not seek to block %ld in temporary file", blknum);
764+
nread = BufFileRead(file, ptr, BLCKSZ);
765+
if (nread != BLCKSZ)
766+
elog(ERROR, "could not read temporary file: read only %zu of %zu bytes",
767+
nread, (size_t) BLCKSZ);
764768
}
765769

766770
static void
767771
WriteTempFileBlock(BufFile *file, long blknum, void *ptr)
768772
{
769773
if (BufFileSeekBlock(file, blknum) != 0)
770-
elog(ERROR, "could not seek temporary file: %m");
771-
if (BufFileWrite(file, ptr, BLCKSZ) != BLCKSZ)
772-
{
773-
/*
774-
* the other errors in Read/WriteTempFileBlock shouldn't happen, but
775-
* an error at write can easily happen if you run out of disk space.
776-
*/
777-
ereport(ERROR,
778-
(errcode_for_file_access(),
779-
errmsg("could not write block %ld of temporary file: %m",
780-
blknum)));
781-
}
774+
elog(ERROR, "could not seek to block %ld in temporary file", blknum);
775+
BufFileWrite(file, ptr, BLCKSZ);
782776
}

‎src/backend/executor/nodeHashjoin.c

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -767,7 +767,7 @@ ExecHashJoinNewBatch(HashJoinState *hjstate)
767767
if (BufFileSeek(innerFile, 0, 0L, SEEK_SET))
768768
ereport(ERROR,
769769
(errcode_for_file_access(),
770-
errmsg("could not rewind hash-join temporary file: %m")));
770+
errmsg("could not rewind hash-join temporary file")));
771771

772772
while ((slot = ExecHashJoinGetSavedTuple(hjstate,
773773
innerFile,
@@ -797,7 +797,7 @@ ExecHashJoinNewBatch(HashJoinState *hjstate)
797797
if (BufFileSeek(hashtable->outerBatchFile[curbatch], 0, 0L, SEEK_SET))
798798
ereport(ERROR,
799799
(errcode_for_file_access(),
800-
errmsg("could not rewind hash-join temporary file: %m")));
800+
errmsg("could not rewind hash-join temporary file")));
801801
}
802802

803803
return true;
@@ -819,7 +819,6 @@ ExecHashJoinSaveTuple(MinimalTuple tuple, uint32 hashvalue,
819819
BufFile **fileptr)
820820
{
821821
BufFile *file = *fileptr;
822-
size_t written;
823822

824823
if (file == NULL)
825824
{
@@ -828,17 +827,8 @@ ExecHashJoinSaveTuple(MinimalTuple tuple, uint32 hashvalue,
828827
*fileptr = file;
829828
}
830829

831-
written = BufFileWrite(file, (void *) &hashvalue, sizeof(uint32));
832-
if (written != sizeof(uint32))
833-
ereport(ERROR,
834-
(errcode_for_file_access(),
835-
errmsg("could not write to hash-join temporary file: %m")));
836-
837-
written = BufFileWrite(file, (void *) tuple, tuple->t_len);
838-
if (written != tuple->t_len)
839-
ereport(ERROR,
840-
(errcode_for_file_access(),
841-
errmsg("could not write to hash-join temporary file: %m")));
830+
BufFileWrite(file, (void *) &hashvalue, sizeof(uint32));
831+
BufFileWrite(file, (void *) tuple, tuple->t_len);
842832
}
843833

844834
/*
@@ -879,7 +869,8 @@ ExecHashJoinGetSavedTuple(HashJoinState *hjstate,
879869
if (nread != sizeof(header))
880870
ereport(ERROR,
881871
(errcode_for_file_access(),
882-
errmsg("could not read from hash-join temporary file: %m")));
872+
errmsg("could not read from hash-join temporary file: read only %zu of %zu bytes",
873+
nread, sizeof(header))));
883874
*hashvalue = header[0];
884875
tuple = (MinimalTuple) palloc(header[1]);
885876
tuple->t_len = header[1];
@@ -889,7 +880,8 @@ ExecHashJoinGetSavedTuple(HashJoinState *hjstate,
889880
if (nread != header[1] - sizeof(uint32))
890881
ereport(ERROR,
891882
(errcode_for_file_access(),
892-
errmsg("could not read from hash-join temporary file: %m")));
883+
errmsg("could not read from hash-join temporary file: read only %zu of %zu bytes",
884+
nread, header[1] - sizeof(uint32))));
893885
return ExecStoreMinimalTuple(tuple, tupleSlot, true);
894886
}
895887

‎src/backend/storage/file/buffile.c

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,7 @@ static BufFile *makeBufFile(File firstfile);
9494
static void extendBufFile(BufFile *file);
9595
static void BufFileLoadBuffer(BufFile *file);
9696
static void BufFileDumpBuffer(BufFile *file);
97-
static int BufFileFlush(BufFile *file);
98-
97+
static void BufFileFlush(BufFile *file);
9998

10099
/*
101100
* Create a BufFile given the first underlying physical file.
@@ -248,7 +247,10 @@ BufFileLoadBuffer(BufFile *file)
248247
if (file->curOffset != file->offsets[file->curFile])
249248
{
250249
if (FileSeek(thisfile, file->curOffset, SEEK_SET) != file->curOffset)
251-
return; /* seek failed, read nothing */
250+
ereport(ERROR,
251+
(errcode_for_file_access(),
252+
errmsg("could not seek in file \"%s\": %m",
253+
FilePathName(thisfile))));
252254
file->offsets[file->curFile] = file->curOffset;
253255
}
254256

@@ -260,7 +262,14 @@ BufFileLoadBuffer(BufFile *file)
260262
sizeof(file->buffer),
261263
WAIT_EVENT_BUFFILE_READ);
262264
if (file->nbytes < 0)
265+
{
263266
file->nbytes = 0;
267+
ereport(ERROR,
268+
(errcode_for_file_access(),
269+
errmsg("could not read file \"%s\": %m",
270+
FilePathName(thisfile))));
271+
}
272+
264273
file->offsets[file->curFile] += file->nbytes;
265274
/* we choose not to advance curOffset here */
266275

@@ -318,15 +327,22 @@ BufFileDumpBuffer(BufFile *file)
318327
if (file->curOffset != file->offsets[file->curFile])
319328
{
320329
if (FileSeek(thisfile, file->curOffset, SEEK_SET) != file->curOffset)
321-
return; /* seek failed, give up */
330+
ereport(ERROR,
331+
(errcode_for_file_access(),
332+
errmsg("could not seek in file \"%s\": %m",
333+
FilePathName(thisfile))));
322334
file->offsets[file->curFile] = file->curOffset;
323335
}
324336
bytestowrite = FileWrite(thisfile,
325337
file->buffer.data + wpos,
326338
bytestowrite,
327339
WAIT_EVENT_BUFFILE_WRITE);
328340
if (bytestowrite <= 0)
329-
return; /* failed to write */
341+
ereport(ERROR,
342+
(errcode_for_file_access(),
343+
errmsg("could not write to file \"%s\" : %m",
344+
FilePathName(thisfile))));
345+
330346
file->offsets[file->curFile] += bytestowrite;
331347
file->curOffset += bytestowrite;
332348
wpos += bytestowrite;
@@ -359,20 +375,16 @@ BufFileDumpBuffer(BufFile *file)
359375
/*
360376
* BufFileRead
361377
*
362-
* Like fread() except we assume 1-byte element size.
378+
* Like fread() except we assume 1-byte element size and report I/O errors via
379+
* ereport().
363380
*/
364381
size_t
365382
BufFileRead(BufFile *file, void *ptr, size_t size)
366383
{
367384
size_t nread = 0;
368385
size_t nthistime;
369386

370-
if (file->dirty)
371-
{
372-
if (BufFileFlush(file) != 0)
373-
return 0; /* could not flush... */
374-
Assert(!file->dirty);
375-
}
387+
BufFileFlush(file);
376388

377389
while (size > 0)
378390
{
@@ -406,7 +418,8 @@ BufFileRead(BufFile *file, void *ptr, size_t size)
406418
/*
407419
* BufFileWrite
408420
*
409-
* Like fwrite() except we assume 1-byte element size.
421+
* Like fwrite() except we assume 1-byte element size and report errors via
422+
* ereport().
410423
*/
411424
size_t
412425
BufFileWrite(BufFile *file, void *ptr, size_t size)
@@ -420,11 +433,7 @@ BufFileWrite(BufFile *file, void *ptr, size_t size)
420433
{
421434
/* Buffer full, dump it out */
422435
if (file->dirty)
423-
{
424436
BufFileDumpBuffer(file);
425-
if (file->dirty)
426-
break; /* I/O error */
427-
}
428437
else
429438
{
430439
/* Hmm, went directly from reading to writing? */
@@ -456,19 +465,15 @@ BufFileWrite(BufFile *file, void *ptr, size_t size)
456465
/*
457466
* BufFileFlush
458467
*
459-
* Like fflush()
468+
* Like fflush(), except that I/O errors are reported with ereport().
460469
*/
461-
static int
470+
static void
462471
BufFileFlush(BufFile *file)
463472
{
464473
if (file->dirty)
465-
{
466474
BufFileDumpBuffer(file);
467-
if (file->dirty)
468-
return EOF;
469-
}
470475

471-
return 0;
476+
Assert(!file->dirty);
472477
}
473478

474479
/*
@@ -477,6 +482,7 @@ BufFileFlush(BufFile *file)
477482
* Like fseek(), except that target position needs two values in order to
478483
* work when logical filesize exceeds maximum value representable by long.
479484
* We do not support relative seeks across more than LONG_MAX, however.
485+
* I/O errors are reported by ereport().
480486
*
481487
* Result is 0 if OK, EOF if not. Logical position is not moved if an
482488
* impossible seek is attempted.
@@ -534,8 +540,7 @@ BufFileSeek(BufFile *file, int fileno, off_t offset, int whence)
534540
return 0;
535541
}
536542
/* Otherwise, must reposition buffer, so flush any dirty data */
537-
if (BufFileFlush(file) != 0)
538-
return EOF;
543+
BufFileFlush(file);
539544

540545
/*
541546
* At this point and no sooner, check for seek past last segment. The

‎src/backend/utils/sort/logtape.c

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -223,12 +223,12 @@ ltsWriteBlock(LogicalTapeSet *lts, long blocknum, void *buffer)
223223
}
224224

225225
/* Write the requested block */
226-
if (BufFileSeekBlock(lts->pfile, blocknum) != 0 ||
227-
BufFileWrite(lts->pfile, buffer, BLCKSZ) != BLCKSZ)
226+
if (BufFileSeekBlock(lts->pfile, blocknum) != 0)
228227
ereport(ERROR,
229228
(errcode_for_file_access(),
230-
errmsg("could not write block %ld of temporary file: %m",
229+
errmsg("could not seek to block %ld of temporary file",
231230
blocknum)));
231+
BufFileWrite(lts->pfile, buffer, BLCKSZ);
232232

233233
/* Update nBlocksWritten, if we extended the file */
234234
if (blocknum == lts->nBlocksWritten)
@@ -244,12 +244,19 @@ ltsWriteBlock(LogicalTapeSet *lts, long blocknum, void *buffer)
244244
static void
245245
ltsReadBlock(LogicalTapeSet *lts, long blocknum, void *buffer)
246246
{
247-
if (BufFileSeekBlock(lts->pfile, blocknum) != 0 ||
248-
BufFileRead(lts->pfile, buffer, BLCKSZ) != BLCKSZ)
247+
size_t nread;
248+
249+
if (BufFileSeekBlock(lts->pfile, blocknum) != 0)
249250
ereport(ERROR,
250251
(errcode_for_file_access(),
251-
errmsg("could not read block %ld of temporary file: %m",
252+
errmsg("could not seek to block %ld of temporary file",
252253
blocknum)));
254+
nread = BufFileRead(lts->pfile, buffer, BLCKSZ);
255+
if (nread != BLCKSZ)
256+
ereport(ERROR,
257+
(errcode_for_file_access(),
258+
errmsg("could not read block %ld of temporary file: read only %zu of %zu bytes",
259+
blocknum, nread, (size_t) BLCKSZ)));
253260
}
254261

255262
/*

0 commit comments

Comments
 (0)