Re: pg_read_file() with virtual files returns empty string

2020-07-04 Thread Joe Conway
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

Re: pg_read_file() with virtual files returns empty string

2020-07-04 Thread Joe Conway
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

Re: pg_read_file() with virtual files returns empty string

2020-07-04 Thread Tom Lane
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); >

Re: pg_read_file() with virtual files returns empty string

2020-07-04 Thread Justin Pryzby
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

Re: pg_read_file() with virtual files returns empty string

2020-07-04 Thread Joe Conway
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

Re: pg_read_file() with virtual files returns empty string

2020-07-02 Thread Tom Lane
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

Re: pg_read_file() with virtual files returns empty string

2020-07-02 Thread Joe Conway
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.: > >

Re: pg_read_file() with virtual files returns empty string

2020-07-02 Thread Tom Lane
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: > +

Re: pg_read_file() with virtual files returns empty string

2020-07-02 Thread Joe Conway
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

Re: pg_read_file() with virtual files returns empty string

2020-07-02 Thread Tom Lane
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.

Re: pg_read_file() with virtual files returns empty string

2020-07-02 Thread Joe Conway
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

Re: pg_read_file() with virtual files returns empty string

2020-07-02 Thread Tom Lane
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

Re: pg_read_file() with virtual files returns empty string

2020-07-02 Thread Joe Conway
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

Re: pg_read_file() with virtual files returns empty string

2020-07-01 Thread Tom Lane
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

Re: pg_read_file() with virtual files returns empty string

2020-07-01 Thread Joe Conway
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

Re: pg_read_file() with virtual files returns empty string

2020-07-01 Thread Joe Conway
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

Re: pg_read_file() with virtual files returns empty string

2020-07-01 Thread Tom Lane
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

Re: pg_read_file() with virtual files returns empty string

2020-06-30 Thread Joe Conway
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

Re: pg_read_file() with virtual files returns empty string

2020-06-28 Thread Tom Lane
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

Re: pg_read_file() with virtual files returns empty string

2020-06-28 Thread Joe Conway
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()

Re: pg_read_file() with virtual files returns empty string

2020-06-27 Thread Tom Lane
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

pg_read_file() with virtual files returns empty string

2020-06-27 Thread Joe Conway
Since pg11 pg_read_file() and friends can be used with absolute paths as long as the user is superuser or explicitly granted the role pg_read_server_files. I noticed that when trying to read a virtual file, e.g.: SELECT pg_read_file('/proc/self/status'); the returned result is a zero length