Add BufFileRead variants with short read and EOF detection

Most callers of BufFileRead() want to check whether they read the full
specified length.  Checking this at every call site is very tedious.
This patch provides additional variants BufFileReadExact() and
BufFileReadMaybeEOF() that include the length checks.

I considered changing BufFileRead() itself, but this function is also
used in extensions, and so changing the behavior like this would
create a lot of problems there.  The new names are analogous to the
existing LogicalTapeReadExact().

Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/f3501945-c591-8cc3-5ef0-b72a2e0eaa9c@enterprisedb.com

3 files changed, 13 insertions, 74 deletions

diff --git a/src/backend/utils/sort/logtape.c b/src/backend/utils/sort/logtape.c
index 907dd8f0a78..56ac0298c55 100644
--- a/src/backend/utils/sort/logtape.c
+++ b/src/backend/utils/sort/logtape.c
@@ -281,19 +281,12 @@ ltsWriteBlock(LogicalTapeSet *lts, long blocknum, const void *buffer)
 static void
 ltsReadBlock(LogicalTapeSet *lts, long blocknum, void *buffer)
 {
-	size_t		nread;
-
 	if (BufFileSeekBlock(lts->pfile, blocknum) != 0)
 		ereport(ERROR,
 				(errcode_for_file_access(),
 				 errmsg("could not seek to block %ld of temporary file",
 						blocknum)));
-	nread = BufFileRead(lts->pfile, buffer, BLCKSZ);
-	if (nread != BLCKSZ)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not read block %ld of temporary file: read only %zu of %zu bytes",
-						blocknum, nread, (size_t) BLCKSZ)));
+	BufFileReadExact(lts->pfile, buffer, BLCKSZ);
 }
 
 /*
diff --git a/src/backend/utils/sort/sharedtuplestore.c b/src/backend/utils/sort/sharedtuplestore.c
index 65bbe0a52d1..e3da83f10be 100644
--- a/src/backend/utils/sort/sharedtuplestore.c
+++ b/src/backend/utils/sort/sharedtuplestore.c
@@ -422,23 +422,10 @@ sts_read_tuple(SharedTuplestoreAccessor *accessor, void *meta_data)
 	 */
 	if (accessor->sts->meta_data_size > 0)
 	{
-		if (BufFileRead(accessor->read_file,
-						meta_data,
-						accessor->sts->meta_data_size) !=
-			accessor->sts->meta_data_size)
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read from shared tuplestore temporary file"),
-					 errdetail_internal("Short read while reading meta-data.")));
+		BufFileReadExact(accessor->read_file, meta_data, accessor->sts->meta_data_size);
 		accessor->read_bytes += accessor->sts->meta_data_size;
 	}
-	if (BufFileRead(accessor->read_file,
-					&size,
-					sizeof(size)) != sizeof(size))
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not read from shared tuplestore temporary file"),
-				 errdetail_internal("Short read while reading size.")));
+	BufFileReadExact(accessor->read_file, &size, sizeof(size));
 	accessor->read_bytes += sizeof(size);
 	if (size > accessor->read_buffer_size)
 	{
@@ -455,13 +442,7 @@ sts_read_tuple(SharedTuplestoreAccessor *accessor, void *meta_data)
 	this_chunk_size = Min(remaining_size,
 						  BLCKSZ * STS_CHUNK_PAGES - accessor->read_bytes);
 	destination = accessor->read_buffer + sizeof(uint32);
-	if (BufFileRead(accessor->read_file,
-					destination,
-					this_chunk_size) != this_chunk_size)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not read from shared tuplestore temporary file"),
-				 errdetail_internal("Short read while reading tuple.")));
+	BufFileReadExact(accessor->read_file, destination, this_chunk_size);
 	accessor->read_bytes += this_chunk_size;
 	remaining_size -= this_chunk_size;
 	destination += this_chunk_size;
@@ -473,12 +454,7 @@ sts_read_tuple(SharedTuplestoreAccessor *accessor, void *meta_data)
 		/* We are now positioned at the start of an overflow chunk. */
 		SharedTuplestoreChunk chunk_header;
 
-		if (BufFileRead(accessor->read_file, &chunk_header, STS_CHUNK_HEADER_SIZE) !=
-			STS_CHUNK_HEADER_SIZE)
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read from shared tuplestore temporary file"),
-					 errdetail_internal("Short read while reading overflow chunk header.")));
+		BufFileReadExact(accessor->read_file, &chunk_header, STS_CHUNK_HEADER_SIZE);
 		accessor->read_bytes = STS_CHUNK_HEADER_SIZE;
 		if (chunk_header.overflow == 0)
 			ereport(ERROR,
@@ -489,13 +465,7 @@ sts_read_tuple(SharedTuplestoreAccessor *accessor, void *meta_data)
 		this_chunk_size = Min(remaining_size,
 							  BLCKSZ * STS_CHUNK_PAGES -
 							  STS_CHUNK_HEADER_SIZE);
-		if (BufFileRead(accessor->read_file,
-						destination,
-						this_chunk_size) != this_chunk_size)
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read from shared tuplestore temporary file"),
-					 errdetail_internal("Short read while reading tuple.")));
+		BufFileReadExact(accessor->read_file, destination, this_chunk_size);
 		accessor->read_bytes += this_chunk_size;
 		remaining_size -= this_chunk_size;
 		destination += this_chunk_size;
@@ -551,7 +521,6 @@ sts_parallel_scan_next(SharedTuplestoreAccessor *accessor, void *meta_data)
 		if (!eof)
 		{
 			SharedTuplestoreChunk chunk_header;
-			size_t		nread;
 
 			/* Make sure we have the file open. */
 			if (accessor->read_file == NULL)
@@ -570,13 +539,7 @@ sts_parallel_scan_next(SharedTuplestoreAccessor *accessor, void *meta_data)
 						(errcode_for_file_access(),
 						 errmsg("could not seek to block %u in shared tuplestore temporary file",
 								read_page)));
-			nread = BufFileRead(accessor->read_file, &chunk_header,
-								STS_CHUNK_HEADER_SIZE);
-			if (nread != STS_CHUNK_HEADER_SIZE)
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not read from shared tuplestore temporary file: read only %zu of %zu bytes",
-								nread, STS_CHUNK_HEADER_SIZE)));
+			BufFileReadExact(accessor->read_file, &chunk_header, STS_CHUNK_HEADER_SIZE);
 
 			/*
 			 * If this is an overflow chunk, we skip it and any following
diff --git a/src/backend/utils/sort/tuplestore.c b/src/backend/utils/sort/tuplestore.c
index 44f1d9e8556..bc36662198a 100644
--- a/src/backend/utils/sort/tuplestore.c
+++ b/src/backend/utils/sort/tuplestore.c
@@ -1468,15 +1468,11 @@ getlen(Tuplestorestate *state, bool eofOK)
 	unsigned int len;
 	size_t		nbytes;
 
-	nbytes = BufFileRead(state->myfile, &len, sizeof(len));
-	if (nbytes == sizeof(len))
+	nbytes = BufFileReadMaybeEOF(state->myfile, &len, sizeof(len), eofOK);
+	if (nbytes == 0)
+		return 0;
+	else
 		return len;
-	if (nbytes != 0 || !eofOK)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not read from tuplestore temporary file: read only %zu of %zu bytes",
-						nbytes, sizeof(len))));
-	return 0;
 }
 
 
@@ -1528,25 +1524,12 @@ readtup_heap(Tuplestorestate *state, unsigned int len)
 	unsigned int tuplen = tupbodylen + MINIMAL_TUPLE_DATA_OFFSET;
 	MinimalTuple tuple = (MinimalTuple) palloc(tuplen);
 	char	   *tupbody = (char *) tuple + MINIMAL_TUPLE_DATA_OFFSET;
-	size_t		nread;
 
 	USEMEM(state, GetMemoryChunkSpace(tuple));
 	/* read in the tuple proper */
 	tuple->t_len = tuplen;
-	nread = BufFileRead(state->myfile, tupbody, tupbodylen);
-	if (nread != (size_t) tupbodylen)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not read from tuplestore temporary file: read only %zu of %zu bytes",
-						nread, (size_t) tupbodylen)));
+	BufFileReadExact(state->myfile, tupbody, tupbodylen);
 	if (state->backward)		/* need trailing length word? */
-	{
-		nread = BufFileRead(state->myfile, &tuplen, sizeof(tuplen));
-		if (nread != sizeof(tuplen))
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read from tuplestore temporary file: read only %zu of %zu bytes",
-							nread, sizeof(tuplen))));
-	}
+		BufFileReadExact(state->myfile, &tuplen, sizeof(tuplen));
 	return (void *) tuple;
 }