On 7/2/20 3:36 PM, Tom Lane wrote: > Joe Conway <m...@joeconway.com> writes: >> On 7/1/20 6:22 PM, Tom Lane wrote: >>> Hm, I was expecting that the last successful iteration of >>> enlargeStringInfo would increase the buffer size to MaxAllocSize, >>> so that we'd really only be losing one byte (which we can't avoid >>> if we use stringinfo). But you're right that it's most likely moot >>> since later manipulations of such a result would risk hitting overflows. > >> Sorry to open this can of worms again, but I couldn't get my head past the >> fact >> that reading the entire file would have a different size limit than reading >> the >> exact number of bytes in the file. > > Are you sure there actually is any such limit in the other code, > after accounting for the way that stringinfo.c will enlarge its > buffer? That is, I believe that the limit is MaxAllocSize minus > five bytes, not something less. > >> So, inspired by what you did (and StringInfo itself) I came up with the >> attached. This version performs equivalently to your patch (and HEAD), and >> allows files up to and including (MaxAllocSize - VARHDRSZ) -- i.e. exactly >> the >> same as the specified-length case and legacy behavior for the full file read. > > I find this way overcomplicated for what it accomplishes. In the > real world there's not much difference between MaxAllocSize minus > five and MaxAllocSize minus four.
Ok, so your version was not as bad as I thought.: ll /tmp/rbftest*.bin -rw-r--r-- 1 postgres postgres 1073741819 Jul 2 15:48 /tmp/rbftest1.bin -rw-r--r-- 1 postgres postgres 1073741818 Jul 2 15:47 /tmp/rbftest2.bin -rw-r--r-- 1 postgres postgres 1073741817 Jul 2 15:53 /tmp/rbftest3.bin rbftest1.bin == MaxAllocSize - 4 rbftest2.bin == MaxAllocSize - 5 rbftest3.bin == MaxAllocSize - 6 postgres=# select length(pg_read_binary_file('/tmp/rbftest1.bin')); ERROR: requested length too large postgres=# select length(pg_read_binary_file('/tmp/rbftest2.bin')); ERROR: requested length too large postgres=# select length(pg_read_binary_file('/tmp/rbftest3.bin')); length ------------ 1073741817 When I saw originally MaxAllocSize - 5 fail I skipped to something smaller by 4096 and it worked. But here I see that the actual max size is MaxAllocSize - 6. I guess I can live with that. Joe -- Crunchy Data - http://crunchydata.com PostgreSQL Support for Secure Enterprises Consulting, Training, & Open Source Development
signature.asc
Description: OpenPGP digital signature