Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 7897e3b

Browse filesBrowse 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 4c5cf54 commit 7897e3b
Copy full SHA for 7897e3b

File tree

Expand file treeCollapse file tree

7 files changed

+88
-110
lines changed
Filter options
Expand file treeCollapse file tree

7 files changed

+88
-110
lines changed

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

Copy file name to clipboardExpand all lines: src/backend/access/gist/gistbuildbuffers.c
+9-15Lines 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

Copy file name to clipboardExpand all lines: src/backend/executor/nodeHashjoin.c
+8-16Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1043,7 +1043,7 @@ ExecHashJoinNewBatch(HashJoinState *hjstate)
10431043
if (BufFileSeek(innerFile, 0, 0L, SEEK_SET))
10441044
ereport(ERROR,
10451045
(errcode_for_file_access(),
1046-
errmsg("could not rewind hash-join temporary file: %m")));
1046+
errmsg("could not rewind hash-join temporary file")));
10471047

10481048
while ((slot = ExecHashJoinGetSavedTuple(hjstate,
10491049
innerFile,
@@ -1073,7 +1073,7 @@ ExecHashJoinNewBatch(HashJoinState *hjstate)
10731073
if (BufFileSeek(hashtable->outerBatchFile[curbatch], 0, 0L, SEEK_SET))
10741074
ereport(ERROR,
10751075
(errcode_for_file_access(),
1076-
errmsg("could not rewind hash-join temporary file: %m")));
1076+
errmsg("could not rewind hash-join temporary file")));
10771077
}
10781078

10791079
return true;
@@ -1219,7 +1219,6 @@ ExecHashJoinSaveTuple(MinimalTuple tuple, uint32 hashvalue,
12191219
BufFile **fileptr)
12201220
{
12211221
BufFile *file = *fileptr;
1222-
size_t written;
12231222

12241223
if (file == NULL)
12251224
{
@@ -1228,17 +1227,8 @@ ExecHashJoinSaveTuple(MinimalTuple tuple, uint32 hashvalue,
12281227
*fileptr = file;
12291228
}
12301229

1231-
written = BufFileWrite(file, (void *) &hashvalue, sizeof(uint32));
1232-
if (written != sizeof(uint32))
1233-
ereport(ERROR,
1234-
(errcode_for_file_access(),
1235-
errmsg("could not write to hash-join temporary file: %m")));
1236-
1237-
written = BufFileWrite(file, (void *) tuple, tuple->t_len);
1238-
if (written != tuple->t_len)
1239-
ereport(ERROR,
1240-
(errcode_for_file_access(),
1241-
errmsg("could not write to hash-join temporary file: %m")));
1230+
BufFileWrite(file, (void *) &hashvalue, sizeof(uint32));
1231+
BufFileWrite(file, (void *) tuple, tuple->t_len);
12421232
}
12431233

12441234
/*
@@ -1279,7 +1269,8 @@ ExecHashJoinGetSavedTuple(HashJoinState *hjstate,
12791269
if (nread != sizeof(header))
12801270
ereport(ERROR,
12811271
(errcode_for_file_access(),
1282-
errmsg("could not read from hash-join temporary file: %m")));
1272+
errmsg("could not read from hash-join temporary file: read only %zu of %zu bytes",
1273+
nread, sizeof(header))));
12831274
*hashvalue = header[0];
12841275
tuple = (MinimalTuple) palloc(header[1]);
12851276
tuple->t_len = header[1];
@@ -1289,7 +1280,8 @@ ExecHashJoinGetSavedTuple(HashJoinState *hjstate,
12891280
if (nread != header[1] - sizeof(uint32))
12901281
ereport(ERROR,
12911282
(errcode_for_file_access(),
1292-
errmsg("could not read from hash-join temporary file: %m")));
1283+
errmsg("could not read from hash-join temporary file: read only %zu of %zu bytes",
1284+
nread, header[1] - sizeof(uint32))));
12931285
ExecForceStoreMinimalTuple(tuple, tupleSlot, true);
12941286
return tupleSlot;
12951287
}

‎src/backend/replication/backup_manifest.c

Copy file name to clipboardExpand all lines: src/backend/replication/backup_manifest.c
+2-7Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ SendBackupManifest(backup_manifest_info *manifest)
319319
if (BufFileSeek(manifest->buffile, 0, 0L, SEEK_SET))
320320
ereport(ERROR,
321321
(errcode_for_file_access(),
322-
errmsg("could not rewind temporary file: %m")));
322+
errmsg("could not rewind temporary file")));
323323

324324
/* Send CopyOutResponse message */
325325
pq_beginmessage(&protobuf, 'H');
@@ -365,15 +365,10 @@ static void
365365
AppendStringToManifest(backup_manifest_info *manifest, char *s)
366366
{
367367
int len = strlen(s);
368-
size_t written;
369368

370369
Assert(manifest != NULL);
371370
if (manifest->still_checksumming)
372371
pg_sha256_update(&manifest->manifest_ctx, (uint8 *) s, len);
373-
written = BufFileWrite(manifest->buffile, s, len);
374-
if (written != len)
375-
ereport(ERROR,
376-
(errcode_for_file_access(),
377-
errmsg("could not write to temporary file: %m")));
372+
BufFileWrite(manifest->buffile, s, len);
378373
manifest->manifest_size += len;
379374
}

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

Copy file name to clipboardExpand all lines: src/backend/storage/file/buffile.c
+22-23Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ static BufFile *makeBufFile(File firstfile);
9999
static void extendBufFile(BufFile *file);
100100
static void BufFileLoadBuffer(BufFile *file);
101101
static void BufFileDumpBuffer(BufFile *file);
102-
static int BufFileFlush(BufFile *file);
102+
static void BufFileFlush(BufFile *file);
103103
static File MakeNewSharedSegment(BufFile *file, int segment);
104104

105105
/*
@@ -434,7 +434,14 @@ BufFileLoadBuffer(BufFile *file)
434434
file->curOffset,
435435
WAIT_EVENT_BUFFILE_READ);
436436
if (file->nbytes < 0)
437+
{
437438
file->nbytes = 0;
439+
ereport(ERROR,
440+
(errcode_for_file_access(),
441+
errmsg("could not read file \"%s\": %m",
442+
FilePathName(thisfile))));
443+
}
444+
438445
/* we choose not to advance curOffset here */
439446

440447
if (file->nbytes > 0)
@@ -490,7 +497,10 @@ BufFileDumpBuffer(BufFile *file)
490497
file->curOffset,
491498
WAIT_EVENT_BUFFILE_WRITE);
492499
if (bytestowrite <= 0)
493-
return; /* failed to write */
500+
ereport(ERROR,
501+
(errcode_for_file_access(),
502+
errmsg("could not write to file \"%s\" : %m",
503+
FilePathName(thisfile))));
494504
file->curOffset += bytestowrite;
495505
wpos += bytestowrite;
496506

@@ -522,20 +532,16 @@ BufFileDumpBuffer(BufFile *file)
522532
/*
523533
* BufFileRead
524534
*
525-
* Like fread() except we assume 1-byte element size.
535+
* Like fread() except we assume 1-byte element size and report I/O errors via
536+
* ereport().
526537
*/
527538
size_t
528539
BufFileRead(BufFile *file, void *ptr, size_t size)
529540
{
530541
size_t nread = 0;
531542
size_t nthistime;
532543

533-
if (file->dirty)
534-
{
535-
if (BufFileFlush(file) != 0)
536-
return 0; /* could not flush... */
537-
Assert(!file->dirty);
538-
}
544+
BufFileFlush(file);
539545

540546
while (size > 0)
541547
{
@@ -569,7 +575,8 @@ BufFileRead(BufFile *file, void *ptr, size_t size)
569575
/*
570576
* BufFileWrite
571577
*
572-
* Like fwrite() except we assume 1-byte element size.
578+
* Like fwrite() except we assume 1-byte element size and report errors via
579+
* ereport().
573580
*/
574581
size_t
575582
BufFileWrite(BufFile *file, void *ptr, size_t size)
@@ -585,11 +592,7 @@ BufFileWrite(BufFile *file, void *ptr, size_t size)
585592
{
586593
/* Buffer full, dump it out */
587594
if (file->dirty)
588-
{
589595
BufFileDumpBuffer(file);
590-
if (file->dirty)
591-
break; /* I/O error */
592-
}
593596
else
594597
{
595598
/* Hmm, went directly from reading to writing? */
@@ -621,19 +624,15 @@ BufFileWrite(BufFile *file, void *ptr, size_t size)
621624
/*
622625
* BufFileFlush
623626
*
624-
* Like fflush()
627+
* Like fflush(), except that I/O errors are reported with ereport().
625628
*/
626-
static int
629+
static void
627630
BufFileFlush(BufFile *file)
628631
{
629632
if (file->dirty)
630-
{
631633
BufFileDumpBuffer(file);
632-
if (file->dirty)
633-
return EOF;
634-
}
635634

636-
return 0;
635+
Assert(!file->dirty);
637636
}
638637

639638
/*
@@ -642,6 +641,7 @@ BufFileFlush(BufFile *file)
642641
* Like fseek(), except that target position needs two values in order to
643642
* work when logical filesize exceeds maximum value representable by off_t.
644643
* We do not support relative seeks across more than that, however.
644+
* I/O errors are reported by ereport().
645645
*
646646
* Result is 0 if OK, EOF if not. Logical position is not moved if an
647647
* impossible seek is attempted.
@@ -699,8 +699,7 @@ BufFileSeek(BufFile *file, int fileno, off_t offset, int whence)
699699
return 0;
700700
}
701701
/* Otherwise, must reposition buffer, so flush any dirty data */
702-
if (BufFileFlush(file) != 0)
703-
return EOF;
702+
BufFileFlush(file);
704703

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

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

Copy file name to clipboardExpand all lines: src/backend/utils/sort/logtape.c
+13-6Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -262,12 +262,12 @@ ltsWriteBlock(LogicalTapeSet *lts, long blocknum, void *buffer)
262262
}
263263

264264
/* Write the requested block */
265-
if (BufFileSeekBlock(lts->pfile, blocknum) != 0 ||
266-
BufFileWrite(lts->pfile, buffer, BLCKSZ) != BLCKSZ)
265+
if (BufFileSeekBlock(lts->pfile, blocknum) != 0)
267266
ereport(ERROR,
268267
(errcode_for_file_access(),
269-
errmsg("could not write block %ld of temporary file: %m",
268+
errmsg("could not seek to block %ld of temporary file",
270269
blocknum)));
270+
BufFileWrite(lts->pfile, buffer, BLCKSZ);
271271

272272
/* Update nBlocksWritten, if we extended the file */
273273
if (blocknum == lts->nBlocksWritten)
@@ -283,12 +283,19 @@ ltsWriteBlock(LogicalTapeSet *lts, long blocknum, void *buffer)
283283
static void
284284
ltsReadBlock(LogicalTapeSet *lts, long blocknum, void *buffer)
285285
{
286-
if (BufFileSeekBlock(lts->pfile, blocknum) != 0 ||
287-
BufFileRead(lts->pfile, buffer, BLCKSZ) != BLCKSZ)
286+
size_t nread;
287+
288+
if (BufFileSeekBlock(lts->pfile, blocknum) != 0)
288289
ereport(ERROR,
289290
(errcode_for_file_access(),
290-
errmsg("could not read block %ld of temporary file: %m",
291+
errmsg("could not seek to block %ld of temporary file",
291292
blocknum)));
293+
nread = BufFileRead(lts->pfile, buffer, BLCKSZ);
294+
if (nread != BLCKSZ)
295+
ereport(ERROR,
296+
(errcode_for_file_access(),
297+
errmsg("could not read block %ld of temporary file: read only %zu of %zu bytes",
298+
blocknum, nread, (size_t) BLCKSZ)));
292299
}
293300

294301
/*

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

Copy file name to clipboardExpand all lines: src/backend/utils/sort/sharedtuplestore.c
+9-12Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -196,14 +196,9 @@ static void
196196
sts_flush_chunk(SharedTuplestoreAccessor *accessor)
197197
{
198198
size_t size;
199-
size_t written;
200199

201200
size = STS_CHUNK_PAGES * BLCKSZ;
202-
written = BufFileWrite(accessor->write_file, accessor->write_chunk, size);
203-
if (written != size)
204-
ereport(ERROR,
205-
(errcode_for_file_access(),
206-
errmsg("could not write to temporary file: %m")));
201+
BufFileWrite(accessor->write_file, accessor->write_chunk, size);
207202
memset(accessor->write_chunk, 0, size);
208203
accessor->write_pointer = &accessor->write_chunk->data[0];
209204
accessor->sts->participants[accessor->participant].npages +=
@@ -555,6 +550,7 @@ sts_parallel_scan_next(SharedTuplestoreAccessor *accessor, void *meta_data)
555550
if (!eof)
556551
{
557552
SharedTuplestoreChunk chunk_header;
553+
size_t nread;
558554

559555
/* Make sure we have the file open. */
560556
if (accessor->read_file == NULL)
@@ -570,14 +566,15 @@ sts_parallel_scan_next(SharedTuplestoreAccessor *accessor, void *meta_data)
570566
if (BufFileSeekBlock(accessor->read_file, read_page) != 0)
571567
ereport(ERROR,
572568
(errcode_for_file_access(),
573-
errmsg("could not read from shared tuplestore temporary file"),
574-
errdetail_internal("Could not seek to next block.")));
575-
if (BufFileRead(accessor->read_file, &chunk_header,
576-
STS_CHUNK_HEADER_SIZE) != STS_CHUNK_HEADER_SIZE)
569+
errmsg("could not seek block %u in shared tuplestore temporary file",
570+
read_page)));
571+
nread = BufFileRead(accessor->read_file, &chunk_header,
572+
STS_CHUNK_HEADER_SIZE);
573+
if (nread != STS_CHUNK_HEADER_SIZE)
577574
ereport(ERROR,
578575
(errcode_for_file_access(),
579-
errmsg("could not read from shared tuplestore temporary file"),
580-
errdetail_internal("Short read while reading chunk header.")));
576+
errmsg("could not read from shared tuplestore temporary file: read only %zu of %zu bytes",
577+
nread, STS_CHUNK_HEADER_SIZE)));
581578

582579
/*
583580
* If this is an overflow chunk, we skip it and any following

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.