Movatterモバイル変換

[0]ホーム
URL:

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

 Sign up
Appearance settings

Commit20428d3

Browse files
committed
Add BufFileRead variants with short read and EOF detection
Most callers of BufFileRead() want to check whether they read the fullspecified length. Checking this at every call site is very tedious.This patch provides additional variants BufFileReadExact() andBufFileReadMaybeEOF() that include the length checks.I considered changing BufFileRead() itself, but this function is alsoused in extensions, and so changing the behavior like this wouldcreate a lot of problems there. The new names are analogous to theexisting LogicalTapeReadExact().Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>Discussion:https://www.postgresql.org/message-id/flat/f3501945-c591-8cc3-5ef0-b72a2e0eaa9c@enterprisedb.com
1 parent1561612 commit20428d3

File tree

9 files changed

+73
-134
lines changed

9 files changed

+73
-134
lines changed

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

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -753,14 +753,9 @@ gistRelocateBuildBuffersOnSplit(GISTBuildBuffers *gfbb, GISTSTATE *giststate,
753753
staticvoid
754754
ReadTempFileBlock(BufFile*file,longblknum,void*ptr)
755755
{
756-
size_tnread;
757-
758756
if (BufFileSeekBlock(file,blknum)!=0)
759757
elog(ERROR,"could not seek to block %ld in temporary file",blknum);
760-
nread=BufFileRead(file,ptr,BLCKSZ);
761-
if (nread!=BLCKSZ)
762-
elog(ERROR,"could not read temporary file: read only %zu of %zu bytes",
763-
nread, (size_t)BLCKSZ);
758+
BufFileReadExact(file,ptr,BLCKSZ);
764759
}
765760

766761
staticvoid

‎src/backend/backup/backup_manifest.c

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -362,17 +362,10 @@ SendBackupManifest(backup_manifest_info *manifest, bbsink *sink)
362362
while (manifest_bytes_done<manifest->manifest_size)
363363
{
364364
size_tbytes_to_read;
365-
size_trc;
366365

367366
bytes_to_read=Min(sink->bbs_buffer_length,
368367
manifest->manifest_size-manifest_bytes_done);
369-
rc=BufFileRead(manifest->buffile,sink->bbs_buffer,
370-
bytes_to_read);
371-
if (rc!=bytes_to_read)
372-
ereport(ERROR,
373-
(errcode_for_file_access(),
374-
errmsg("could not read from temporary file: read only %zu of %zu bytes",
375-
rc,bytes_to_read)));
368+
BufFileReadExact(manifest->buffile,sink->bbs_buffer,bytes_to_read);
376369
bbsink_manifest_contents(sink,bytes_to_read);
377370
manifest_bytes_done+=bytes_to_read;
378371
}

‎src/backend/executor/nodeHashjoin.c

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1260,28 +1260,18 @@ ExecHashJoinGetSavedTuple(HashJoinState *hjstate,
12601260
* we can read them both in one BufFileRead() call without any type
12611261
* cheating.
12621262
*/
1263-
nread=BufFileRead(file,header,sizeof(header));
1263+
nread=BufFileReadMaybeEOF(file,header,sizeof(header), true);
12641264
if (nread==0)/* end of file */
12651265
{
12661266
ExecClearTuple(tupleSlot);
12671267
returnNULL;
12681268
}
1269-
if (nread!=sizeof(header))
1270-
ereport(ERROR,
1271-
(errcode_for_file_access(),
1272-
errmsg("could not read from hash-join temporary file: read only %zu of %zu bytes",
1273-
nread,sizeof(header))));
12741269
*hashvalue=header[0];
12751270
tuple= (MinimalTuple)palloc(header[1]);
12761271
tuple->t_len=header[1];
1277-
nread=BufFileRead(file,
1278-
((char*)tuple+sizeof(uint32)),
1279-
header[1]-sizeof(uint32));
1280-
if (nread!=header[1]-sizeof(uint32))
1281-
ereport(ERROR,
1282-
(errcode_for_file_access(),
1283-
errmsg("could not read from hash-join temporary file: read only %zu of %zu bytes",
1284-
nread,header[1]-sizeof(uint32))));
1272+
BufFileReadExact(file,
1273+
(char*)tuple+sizeof(uint32),
1274+
header[1]-sizeof(uint32));
12851275
ExecForceStoreMinimalTuple(tuple,tupleSlot, true);
12861276
returntupleSlot;
12871277
}

‎src/backend/replication/logical/worker.c

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2069,19 +2069,13 @@ apply_spooled_messages(FileSet *stream_fileset, TransactionId xid,
20692069
CHECK_FOR_INTERRUPTS();
20702070

20712071
/* read length of the on-disk record */
2072-
nbytes=BufFileRead(stream_fd,&len,sizeof(len));
2072+
nbytes=BufFileReadMaybeEOF(stream_fd,&len,sizeof(len), true);
20732073

20742074
/* have we reached end of the file? */
20752075
if (nbytes==0)
20762076
break;
20772077

20782078
/* do we have a correct length? */
2079-
if (nbytes!=sizeof(len))
2080-
ereport(ERROR,
2081-
(errcode_for_file_access(),
2082-
errmsg("could not read from streaming transaction's changes file \"%s\": read only %zu of %zu bytes",
2083-
path,nbytes,sizeof(len))));
2084-
20852079
if (len <=0)
20862080
elog(ERROR,"incorrect length %d in streaming transaction's changes file \"%s\"",
20872081
len,path);
@@ -2090,12 +2084,7 @@ apply_spooled_messages(FileSet *stream_fileset, TransactionId xid,
20902084
buffer=repalloc(buffer,len);
20912085

20922086
/* and finally read the data into the buffer */
2093-
nbytes=BufFileRead(stream_fd,buffer,len);
2094-
if (nbytes!=len)
2095-
ereport(ERROR,
2096-
(errcode_for_file_access(),
2097-
errmsg("could not read from streaming transaction's changes file \"%s\": read only %zu of %zu bytes",
2098-
path,nbytes, (size_t)len)));
2087+
BufFileReadExact(stream_fd,buffer,len);
20992088

21002089
BufFileTell(stream_fd,&fileno,&offset);
21012090

@@ -3993,7 +3982,6 @@ static void
39933982
subxact_info_read(Oidsubid,TransactionIdxid)
39943983
{
39953984
charpath[MAXPGPATH];
3996-
size_tnread;
39973985
Sizelen;
39983986
BufFile*fd;
39993987
MemoryContextoldctx;
@@ -4013,12 +4001,7 @@ subxact_info_read(Oid subid, TransactionId xid)
40134001
return;
40144002

40154003
/* read number of subxact items */
4016-
nread=BufFileRead(fd,&subxact_data.nsubxacts,sizeof(subxact_data.nsubxacts));
4017-
if (nread!=sizeof(subxact_data.nsubxacts))
4018-
ereport(ERROR,
4019-
(errcode_for_file_access(),
4020-
errmsg("could not read from streaming transaction's subxact file \"%s\": read only %zu of %zu bytes",
4021-
path,nread,sizeof(subxact_data.nsubxacts))));
4004+
BufFileReadExact(fd,&subxact_data.nsubxacts,sizeof(subxact_data.nsubxacts));
40224005

40234006
len=sizeof(SubXactInfo)*subxact_data.nsubxacts;
40244007

@@ -4037,14 +4020,7 @@ subxact_info_read(Oid subid, TransactionId xid)
40374020
MemoryContextSwitchTo(oldctx);
40384021

40394022
if (len>0)
4040-
{
4041-
nread=BufFileRead(fd,subxact_data.subxacts,len);
4042-
if (nread!=len)
4043-
ereport(ERROR,
4044-
(errcode_for_file_access(),
4045-
errmsg("could not read from streaming transaction's subxact file \"%s\": read only %zu of %zu bytes",
4046-
path,nread,len)));
4047-
}
4023+
BufFileReadExact(fd,subxact_data.subxacts,len);
40484024

40494025
BufFileClose(fd);
40504026
}

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

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -573,14 +573,19 @@ BufFileDumpBuffer(BufFile *file)
573573
}
574574

575575
/*
576-
* BufFileRead
576+
* BufFileRead variants
577577
*
578578
* Like fread() except we assume 1-byte element size and report I/O errors via
579579
* ereport().
580+
*
581+
* If 'exact' is true, then an error is also raised if the number of bytes
582+
* read is not exactly 'size' (no short reads). If 'exact' and 'eofOK' are
583+
* true, then reading zero bytes is ok.
580584
*/
581-
size_t
582-
BufFileRead(BufFile*file,void*ptr,size_tsize)
585+
staticsize_t
586+
BufFileReadCommon(BufFile*file,void*ptr,size_tsize,boolexact,booleofOK)
583587
{
588+
size_tstart_size=size;
584589
size_tnread=0;
585590
size_tnthistime;
586591

@@ -612,9 +617,48 @@ BufFileRead(BufFile *file, void *ptr, size_t size)
612617
nread+=nthistime;
613618
}
614619

620+
if (exact&&
621+
(nread!=start_size&& !(nread==0&&eofOK)))
622+
ereport(ERROR,
623+
errcode_for_file_access(),
624+
file->name ?
625+
errmsg("could not read from file set \"%s\": read only %zu of %zu bytes",
626+
file->name,nread,start_size) :
627+
errmsg("could not read from temporary file: read only %zu of %zu bytes",
628+
nread,start_size));
629+
615630
returnnread;
616631
}
617632

633+
/*
634+
* Legacy interface where the caller needs to check for end of file or short
635+
* reads.
636+
*/
637+
size_t
638+
BufFileRead(BufFile*file,void*ptr,size_tsize)
639+
{
640+
returnBufFileReadCommon(file,ptr,size, false, false);
641+
}
642+
643+
/*
644+
* Require read of exactly the specified size.
645+
*/
646+
void
647+
BufFileReadExact(BufFile*file,void*ptr,size_tsize)
648+
{
649+
BufFileReadCommon(file,ptr,size, true, false);
650+
}
651+
652+
/*
653+
* Require read of exactly the specified size, but optionally allow end of
654+
* file (in which case 0 is returned).
655+
*/
656+
size_t
657+
BufFileReadMaybeEOF(BufFile*file,void*ptr,size_tsize,booleofOK)
658+
{
659+
returnBufFileReadCommon(file,ptr,size, true,eofOK);
660+
}
661+
618662
/*
619663
* BufFileWrite
620664
*

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -281,19 +281,12 @@ ltsWriteBlock(LogicalTapeSet *lts, long blocknum, const void *buffer)
281281
staticvoid
282282
ltsReadBlock(LogicalTapeSet*lts,longblocknum,void*buffer)
283283
{
284-
size_tnread;
285-
286284
if (BufFileSeekBlock(lts->pfile,blocknum)!=0)
287285
ereport(ERROR,
288286
(errcode_for_file_access(),
289287
errmsg("could not seek to block %ld of temporary file",
290288
blocknum)));
291-
nread=BufFileRead(lts->pfile,buffer,BLCKSZ);
292-
if (nread!=BLCKSZ)
293-
ereport(ERROR,
294-
(errcode_for_file_access(),
295-
errmsg("could not read block %ld of temporary file: read only %zu of %zu bytes",
296-
blocknum,nread, (size_t)BLCKSZ)));
289+
BufFileReadExact(lts->pfile,buffer,BLCKSZ);
297290
}
298291

299292
/*

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

Lines changed: 6 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -422,23 +422,10 @@ sts_read_tuple(SharedTuplestoreAccessor *accessor, void *meta_data)
422422
*/
423423
if (accessor->sts->meta_data_size>0)
424424
{
425-
if (BufFileRead(accessor->read_file,
426-
meta_data,
427-
accessor->sts->meta_data_size)!=
428-
accessor->sts->meta_data_size)
429-
ereport(ERROR,
430-
(errcode_for_file_access(),
431-
errmsg("could not read from shared tuplestore temporary file"),
432-
errdetail_internal("Short read while reading meta-data.")));
425+
BufFileReadExact(accessor->read_file,meta_data,accessor->sts->meta_data_size);
433426
accessor->read_bytes+=accessor->sts->meta_data_size;
434427
}
435-
if (BufFileRead(accessor->read_file,
436-
&size,
437-
sizeof(size))!=sizeof(size))
438-
ereport(ERROR,
439-
(errcode_for_file_access(),
440-
errmsg("could not read from shared tuplestore temporary file"),
441-
errdetail_internal("Short read while reading size.")));
428+
BufFileReadExact(accessor->read_file,&size,sizeof(size));
442429
accessor->read_bytes+=sizeof(size);
443430
if (size>accessor->read_buffer_size)
444431
{
@@ -455,13 +442,7 @@ sts_read_tuple(SharedTuplestoreAccessor *accessor, void *meta_data)
455442
this_chunk_size=Min(remaining_size,
456443
BLCKSZ*STS_CHUNK_PAGES-accessor->read_bytes);
457444
destination=accessor->read_buffer+sizeof(uint32);
458-
if (BufFileRead(accessor->read_file,
459-
destination,
460-
this_chunk_size)!=this_chunk_size)
461-
ereport(ERROR,
462-
(errcode_for_file_access(),
463-
errmsg("could not read from shared tuplestore temporary file"),
464-
errdetail_internal("Short read while reading tuple.")));
445+
BufFileReadExact(accessor->read_file,destination,this_chunk_size);
465446
accessor->read_bytes+=this_chunk_size;
466447
remaining_size-=this_chunk_size;
467448
destination+=this_chunk_size;
@@ -473,12 +454,7 @@ sts_read_tuple(SharedTuplestoreAccessor *accessor, void *meta_data)
473454
/* We are now positioned at the start of an overflow chunk. */
474455
SharedTuplestoreChunkchunk_header;
475456

476-
if (BufFileRead(accessor->read_file,&chunk_header,STS_CHUNK_HEADER_SIZE)!=
477-
STS_CHUNK_HEADER_SIZE)
478-
ereport(ERROR,
479-
(errcode_for_file_access(),
480-
errmsg("could not read from shared tuplestore temporary file"),
481-
errdetail_internal("Short read while reading overflow chunk header.")));
457+
BufFileReadExact(accessor->read_file,&chunk_header,STS_CHUNK_HEADER_SIZE);
482458
accessor->read_bytes=STS_CHUNK_HEADER_SIZE;
483459
if (chunk_header.overflow==0)
484460
ereport(ERROR,
@@ -489,13 +465,7 @@ sts_read_tuple(SharedTuplestoreAccessor *accessor, void *meta_data)
489465
this_chunk_size=Min(remaining_size,
490466
BLCKSZ*STS_CHUNK_PAGES-
491467
STS_CHUNK_HEADER_SIZE);
492-
if (BufFileRead(accessor->read_file,
493-
destination,
494-
this_chunk_size)!=this_chunk_size)
495-
ereport(ERROR,
496-
(errcode_for_file_access(),
497-
errmsg("could not read from shared tuplestore temporary file"),
498-
errdetail_internal("Short read while reading tuple.")));
468+
BufFileReadExact(accessor->read_file,destination,this_chunk_size);
499469
accessor->read_bytes+=this_chunk_size;
500470
remaining_size-=this_chunk_size;
501471
destination+=this_chunk_size;
@@ -551,7 +521,6 @@ sts_parallel_scan_next(SharedTuplestoreAccessor *accessor, void *meta_data)
551521
if (!eof)
552522
{
553523
SharedTuplestoreChunkchunk_header;
554-
size_tnread;
555524

556525
/* Make sure we have the file open. */
557526
if (accessor->read_file==NULL)
@@ -570,13 +539,7 @@ sts_parallel_scan_next(SharedTuplestoreAccessor *accessor, void *meta_data)
570539
(errcode_for_file_access(),
571540
errmsg("could not seek to block %u in shared tuplestore temporary file",
572541
read_page)));
573-
nread=BufFileRead(accessor->read_file,&chunk_header,
574-
STS_CHUNK_HEADER_SIZE);
575-
if (nread!=STS_CHUNK_HEADER_SIZE)
576-
ereport(ERROR,
577-
(errcode_for_file_access(),
578-
errmsg("could not read from shared tuplestore temporary file: read only %zu of %zu bytes",
579-
nread,STS_CHUNK_HEADER_SIZE)));
542+
BufFileReadExact(accessor->read_file,&chunk_header,STS_CHUNK_HEADER_SIZE);
580543

581544
/*
582545
* If this is an overflow chunk, we skip it and any following

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

Lines changed: 6 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1468,15 +1468,11 @@ getlen(Tuplestorestate *state, bool eofOK)
14681468
unsignedintlen;
14691469
size_tnbytes;
14701470

1471-
nbytes=BufFileRead(state->myfile,&len,sizeof(len));
1472-
if (nbytes==sizeof(len))
1471+
nbytes=BufFileReadMaybeEOF(state->myfile,&len,sizeof(len),eofOK);
1472+
if (nbytes==0)
1473+
return0;
1474+
else
14731475
returnlen;
1474-
if (nbytes!=0|| !eofOK)
1475-
ereport(ERROR,
1476-
(errcode_for_file_access(),
1477-
errmsg("could not read from tuplestore temporary file: read only %zu of %zu bytes",
1478-
nbytes,sizeof(len))));
1479-
return0;
14801476
}
14811477

14821478

@@ -1528,25 +1524,12 @@ readtup_heap(Tuplestorestate *state, unsigned int len)
15281524
unsignedinttuplen=tupbodylen+MINIMAL_TUPLE_DATA_OFFSET;
15291525
MinimalTupletuple= (MinimalTuple)palloc(tuplen);
15301526
char*tupbody= (char*)tuple+MINIMAL_TUPLE_DATA_OFFSET;
1531-
size_tnread;
15321527

15331528
USEMEM(state,GetMemoryChunkSpace(tuple));
15341529
/* read in the tuple proper */
15351530
tuple->t_len=tuplen;
1536-
nread=BufFileRead(state->myfile,tupbody,tupbodylen);
1537-
if (nread!= (size_t)tupbodylen)
1538-
ereport(ERROR,
1539-
(errcode_for_file_access(),
1540-
errmsg("could not read from tuplestore temporary file: read only %zu of %zu bytes",
1541-
nread, (size_t)tupbodylen)));
1531+
BufFileReadExact(state->myfile,tupbody,tupbodylen);
15421532
if (state->backward)/* need trailing length word? */
1543-
{
1544-
nread=BufFileRead(state->myfile,&tuplen,sizeof(tuplen));
1545-
if (nread!=sizeof(tuplen))
1546-
ereport(ERROR,
1547-
(errcode_for_file_access(),
1548-
errmsg("could not read from tuplestore temporary file: read only %zu of %zu bytes",
1549-
nread,sizeof(tuplen))));
1550-
}
1533+
BufFileReadExact(state->myfile,&tuplen,sizeof(tuplen));
15511534
return (void*)tuple;
15521535
}

‎src/include/storage/buffile.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,9 @@ typedef struct BufFile BufFile;
3838

3939
externBufFile*BufFileCreateTemp(boolinterXact);
4040
externvoidBufFileClose(BufFile*file);
41-
externsize_tBufFileRead(BufFile*file,void*ptr,size_tsize);
41+
externpg_nodiscardsize_tBufFileRead(BufFile*file,void*ptr,size_tsize);
42+
externvoidBufFileReadExact(BufFile*file,void*ptr,size_tsize);
43+
externsize_tBufFileReadMaybeEOF(BufFile*file,void*ptr,size_tsize,booleofOK);
4244
externvoidBufFileWrite(BufFile*file,constvoid*ptr,size_tsize);
4345
externintBufFileSeek(BufFile*file,intfileno,off_toffset,intwhence);
4446
externvoidBufFileTell(BufFile*file,int*fileno,off_t*offset);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp