On 7/4/20 1:10 PM, Joe Conway wrote:
> On 7/4/20 12:52 PM, Tom Lane wrote:
>> Justin Pryzby writes:
>>> But I noticed that cfbot is now populating with failures like:
>>
>>> genfile.c: In function ‘read_binary_file’:
>>> genfile.c:192:5: error: ignoring return value of ‘fread’, declared with
>>>
On 7/4/20 12:52 PM, Tom Lane wrote:
> Justin Pryzby writes:
>> But I noticed that cfbot is now populating with failures like:
>
>> genfile.c: In function ‘read_binary_file’:
>> genfile.c:192:5: error: ignoring return value of ‘fread’, declared with
>> attribute warn_unused_result [-Werror=unused
Justin Pryzby writes:
> But I noticed that cfbot is now populating with failures like:
> genfile.c: In function ‘read_binary_file’:
> genfile.c:192:5: error: ignoring return value of ‘fread’, declared with
> attribute warn_unused_result [-Werror=unused-result]
> fread(rbuf, 1, 1, file);
>
Hi Joe
Thanks for addressing this.
But I noticed that cfbot is now populating with failures like:
https://travis-ci.org/github/postgresql-cfbot/postgresql/builds/704898559
genfile.c: In function ‘read_binary_file’:
genfile.c:192:5: error: ignoring return value of ‘fread’, declared with
attribut
On 7/2/20 6:29 PM, Tom Lane wrote:
> Joe Conway writes:
>> On 7/2/20 5:37 PM, Tom Lane wrote:
>>> 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.
>
> Personally I would not
Joe Conway writes:
> On 7/2/20 5:37 PM, Tom Lane wrote:
>> 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.
Personally I would not bother, but it's your patch.
> Are we in agreem
On 7/2/20 5:37 PM, Tom Lane wrote:
> Joe Conway 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.:
>
>
Joe Conway writes:
> On 7/2/20 4:27 PM, Tom Lane wrote:
>> Huh, I wonder why it's not max - 5. Probably not worth worrying about,
>> though.
> Well this part:
> + rbytes = fread(sbuf.data + sbuf.len, 1,
> +(size_t) (sbuf.maxlen - sbuf.len - 1), file);
> could actually be:
> + rb
On 7/2/20 4:27 PM, Tom Lane wrote:
> Joe Conway writes:
>> 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.
>
> Huh, I wonder why it's not max - 5. Probably not worth worrying abo
Joe Conway writes:
> 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.
Huh, I wonder why it's not max - 5. Probably not worth worrying about,
though.
regards
On 7/2/20 3:36 PM, Tom Lane wrote:
> Joe Conway 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
>>
Joe Conway 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
On 7/1/20 6:22 PM, Tom Lane wrote:
> Joe Conway writes:
>> The only downside is that the max filesize is reduced to (MaxAllocSize -
>> MIN_READ_SIZE - 1) compared to MaxAllocSize with the old method.
>
> Hm, I was expecting that the last successful iteration of
> enlargeStringInfo would increase
Joe Conway writes:
> The only downside is that the max filesize is reduced to (MaxAllocSize -
> MIN_READ_SIZE - 1) compared to MaxAllocSize with the old method.
Hm, I was expecting that the last successful iteration of
enlargeStringInfo would increase the buffer size to MaxAllocSize,
so that we'd
On 7/1/20 5:17 PM, Joe Conway wrote:
> On 7/1/20 4:12 PM, Tom Lane wrote:
>> Joe Conway writes:
>>> I did some performance testing of the worst case/largest possible file and
>>> found
>>> that skipping the stat and bulk read does cause a significant regression.
>>
>> Yeah, I was wondering a lit
On 7/1/20 4:12 PM, Tom Lane wrote:
> Joe Conway writes:
>> I did some performance testing of the worst case/largest possible file and
>> found
>> that skipping the stat and bulk read does cause a significant regression.
>
> Yeah, I was wondering a little bit if that'd be an issue.
>
>> In the a
Joe Conway writes:
> I did some performance testing of the worst case/largest possible file and
> found
> that skipping the stat and bulk read does cause a significant regression.
Yeah, I was wondering a little bit if that'd be an issue.
> In the attached patch I was able to get most of the per
On 6/28/20 6:00 PM, Tom Lane wrote:
> Joe Conway writes:
>> All good stuff -- I believe the attached checks all the boxes.
>
> Looks okay to me, except I think you want
>
> ! if (bytes_to_read > 0)
>
> to be
>
> ! if (bytes_to_read >= 0)
Yep -- thanks.
I did some performance testing
Joe Conway writes:
> All good stuff -- I believe the attached checks all the boxes.
Looks okay to me, except I think you want
! if (bytes_to_read > 0)
to be
! if (bytes_to_read >= 0)
As it stands, a zero request will be treated like -1 (read all the
rest of the file) while ISTM it
On 6/27/20 3:43 PM, Tom Lane wrote:
> Joe Conway writes:
>> The attached patch fixes this for me. I think it ought to be backpatched
>> through
>> pg11.
>
>> Comments?
>
> 1. Doesn't seem to be accounting for the possibility of an error in fread().
>
> 2. Don't we want to remove the stat() cal
Joe Conway writes:
> The attached patch fixes this for me. I think it ought to be backpatched
> through
> pg11.
> Comments?
1. Doesn't seem to be accounting for the possibility of an error in fread().
2. Don't we want to remove the stat() call altogether, if we're not
going to believe its leng
21 matches
Mail list logo
