On 7/2/20 5:37 PM, Tom Lane wrote: > Joe Conway <m...@joeconway.com> writes: >> In fact, in principle there is no reason we can't get to max - 4 with this >> code >> except that when the filesize is exactly 1073741819, we need to try to read >> one >> more byte to find the EOF that way I did in my patch. I.e.: > > Ah, right, *that* is where the extra byte is lost: we need a buffer > workspace one byte more than the file size, or we won't ever actually > see the EOF indication. > > I still can't get excited about contorting the code to remove that > issue.
It doesn't seem much worse than the oom test that was there before -- see attached. In any case I will give you the last word and then quit bugging you about it ;-) Are we in agreement that whatever gets pushed should be backpatched through pg11 (see start of thread)? Joe -- Crunchy Data - http://crunchydata.com PostgreSQL Support for Secure Enterprises Consulting, Training, & Open Source Development
diff --git a/contrib/adminpack/expected/adminpack.out b/contrib/adminpack/expected/adminpack.out
index 5738b0f..edf3ebf 100644
*** a/contrib/adminpack/expected/adminpack.out
--- b/contrib/adminpack/expected/adminpack.out
*************** SELECT pg_file_rename('test_file1', 'tes
*** 79,85 ****
(1 row)
SELECT pg_read_file('test_file1'); -- not there
! ERROR: could not stat file "test_file1": No such file or directory
SELECT pg_read_file('test_file2');
pg_read_file
--------------
--- 79,85 ----
(1 row)
SELECT pg_read_file('test_file1'); -- not there
! ERROR: could not open file "test_file1" for reading: No such file or directory
SELECT pg_read_file('test_file2');
pg_read_file
--------------
*************** SELECT pg_file_rename('test_file2', 'tes
*** 108,114 ****
(1 row)
SELECT pg_read_file('test_file2'); -- not there
! ERROR: could not stat file "test_file2": No such file or directory
SELECT pg_read_file('test_file3');
pg_read_file
--------------
--- 108,114 ----
(1 row)
SELECT pg_read_file('test_file2'); -- not there
! ERROR: could not open file "test_file2" for reading: No such file or directory
SELECT pg_read_file('test_file3');
pg_read_file
--------------
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index ceaa618..32a5eab 100644
*** a/src/backend/utils/adt/genfile.c
--- b/src/backend/utils/adt/genfile.c
*************** read_binary_file(const char *filename, i
*** 106,138 ****
bool missing_ok)
{
bytea *buf;
! size_t nbytes;
FILE *file;
! if (bytes_to_read < 0)
! {
! if (seek_offset < 0)
! bytes_to_read = -seek_offset;
! else
! {
! struct stat fst;
!
! if (stat(filename, &fst) < 0)
! {
! if (missing_ok && errno == ENOENT)
! return NULL;
! else
! ereport(ERROR,
! (errcode_for_file_access(),
! errmsg("could not stat file \"%s\": %m", filename)));
! }
!
! bytes_to_read = fst.st_size - seek_offset;
! }
! }
!
! /* not sure why anyone thought that int64 length was a good idea */
! if (bytes_to_read > (MaxAllocSize - VARHDRSZ))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("requested length too large")));
--- 106,116 ----
bool missing_ok)
{
bytea *buf;
! size_t nbytes = 0;
FILE *file;
! /* clamp request size to what we can actually deliver */
! if (bytes_to_read > (int64) (MaxAllocSize - VARHDRSZ))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("requested length too large")));
*************** read_binary_file(const char *filename, i
*** 154,162 ****
(errcode_for_file_access(),
errmsg("could not seek in file \"%s\": %m", filename)));
! buf = (bytea *) palloc((Size) bytes_to_read + VARHDRSZ);
! nbytes = fread(VARDATA(buf), 1, (size_t) bytes_to_read, file);
if (ferror(file))
ereport(ERROR,
--- 132,199 ----
(errcode_for_file_access(),
errmsg("could not seek in file \"%s\": %m", filename)));
! if (bytes_to_read >= 0)
! {
! /* If passed explicit read size just do it */
! buf = (bytea *) palloc((Size) bytes_to_read + VARHDRSZ);
! nbytes = fread(VARDATA(buf), 1, (size_t) bytes_to_read, file);
! }
! else
! {
! /* Negative read size, read rest of file */
! StringInfoData sbuf;
!
! initStringInfo(&sbuf);
! /* Leave room in the buffer for the varlena length word */
! sbuf.len += VARHDRSZ;
! Assert(sbuf.len < sbuf.maxlen);
!
! while (!(feof(file) || ferror(file)))
! {
! size_t rbytes;
!
! /* Minimum amount to read at a time */
! #define MIN_READ_SIZE 4096
!
! /*
! * If not at end of file, and sbuf.len is equal to
! * MaxAllocSize - 1, then either the file is too large, or
! * there is nothing left to read. Attempt to read one more
! * byte to see if the end of file has been reached. If not,
! * the file is too large; we'd rather give the error message
! * for that ourselves.
! */
! if (sbuf.len == MaxAllocSize - 1)
! {
! char rbuf[1];
!
! fread(rbuf, 1, 1, file);
! if (!feof(file))
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("requested length too large")));
! else
! break;
! }
!
! /* OK, ensure that we can read at least MIN_READ_SIZE */
! enlargeStringInfo(&sbuf, MIN_READ_SIZE);
!
! /*
! * stringinfo.c likes to allocate in powers of 2, so it's likely
! * that much more space is available than we asked for. Use all
! * of it, rather than making more fread calls than necessary.
! */
! rbytes = fread(sbuf.data + sbuf.len, 1,
! (size_t) (sbuf.maxlen - sbuf.len - 1), file);
! sbuf.len += rbytes;
! nbytes += rbytes;
! }
!
! /* Now we can commandeer the stringinfo's buffer as the result */
! buf = (bytea *) sbuf.data;
! }
if (ferror(file))
ereport(ERROR,
signature.asc
Description: OpenPGP digital signature
