On 7/2/20 3:36 PM, Tom Lane wrote:
> Joe Conway <mail@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