Re: [proposal] de-TOAST'ing using a iterator

2020-11-27 Thread Anastasia Lubennikova
On 02.11.2020 22:08, John Naylor wrote: On Mon, Nov 2, 2020 at 1:30 PM Alvaro Herrera > wrote: On 2020-Nov-02, Anastasia Lubennikova wrote: > Status update for a commitfest entry. > > This entry was inactive for a very long time. > John, ar

Re: [proposal] de-TOAST'ing using a iterator

2020-11-02 Thread John Naylor
On Mon, Nov 2, 2020 at 1:30 PM Alvaro Herrera wrote: > On 2020-Nov-02, Anastasia Lubennikova wrote: > > > Status update for a commitfest entry. > > > > This entry was inactive for a very long time. > > John, are you going to continue working on this? > Not in the near future. For background, thi

Re: [proposal] de-TOAST'ing using a iterator

2020-11-02 Thread Alvaro Herrera
On 2020-Nov-02, Anastasia Lubennikova wrote: > Status update for a commitfest entry. > > This entry was inactive for a very long time. > John, are you going to continue working on this? > > The last message mentions some open issues, namely backend crashes, so I move > it to "Waiting on author

Re: [proposal] de-TOAST'ing using a iterator

2020-11-02 Thread Anastasia Lubennikova
Status update for a commitfest entry. This entry was inactive for a very long time. John, are you going to continue working on this? The last message mentions some open issues, namely backend crashes, so I move it to "Waiting on author". The new status of this patch is: Waiting on Author

Re: [proposal] de-TOAST'ing using a iterator

2020-03-25 Thread John Naylor
On Fri, Mar 13, 2020 at 10:19 PM Alvaro Herrera wrote: > > So let's move forward with v10 (submitted on Sept 10th). In the attached v12, based on v10, I've made some progress to address some of the remaining issues. There's still some work to be done, in particular to think about how to hide the

Re: [proposal] de-TOAST'ing using a iterator

2020-03-13 Thread Alvaro Herrera
On 2020-Jan-12, John Naylor wrote: > > I took a brief look at v11 to see if there's anything I can do to help > it move forward. I'm not yet sure how it would look code-wise to > implement Alvaro and Tomas's comments upthread, but I'm pretty sure > this part means the iterator-related structs are

Re: [proposal] de-TOAST'ing using a iterator

2020-01-11 Thread John Naylor
On Mon, Sep 23, 2019 at 9:55 PM Binguo Bao wrote: > > Alvaro Herrera 于2019年9月17日周二 上午5:51写道: >> In broad terms this patch looks pretty good to me. I only have a small >> quibble with this API definition in fmgr.h -- namely that it forces us >> to export the definition of all the structs (that co

Re: [proposal] de-TOAST'ing using a iterator

2019-11-27 Thread Michael Paquier
On Mon, Sep 23, 2019 at 09:55:24PM +0800, Binguo Bao wrote: > Personally, I prefer patch v10. Its performance is superior, although it > exposes some struct details. Please be careful. The patch was waiting for author input, but its latest status does not match what the CF app was saying. I have

Re: [proposal] de-TOAST'ing using a iterator

2019-09-25 Thread Paul Ramsey
> On Sep 23, 2019, at 9:45 AM, Alvaro Herrera wrote: > > Paul Ramsey, do you have opinions to share about this patch? I think > PostGIS might benefit from it. Thread starts here: I like the idea a great deal, but actually PostGIS is probably neutral on it: we generally want to retrieve thi

Re: [proposal] de-TOAST'ing using a iterator

2019-09-23 Thread Alvaro Herrera
Paul Ramsey, do you have opinions to share about this patch? I think PostGIS might benefit from it. Thread starts here: https://postgr.es/m/cal-ogks_onzpc9m9bxpcztmofwulcfkyecekiagxzwrl8kx...@mail.gmail.com -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 2

Re: [proposal] de-TOAST'ing using a iterator

2019-09-23 Thread Binguo Bao
Alvaro Herrera 于2019年9月17日周二 上午5:51写道: > On 2019-Sep-10, Binguo Bao wrote: > > > +/* > > + * Support for de-TOASTing toasted value iteratively. "need" is a > pointer > > + * between the beginning and end of iterator's ToastBuffer. The marco > > + * de-TOAST all bytes before "need" into iterator's

Re: [proposal] de-TOAST'ing using a iterator

2019-09-17 Thread Tomas Vondra
On Mon, Sep 16, 2019 at 06:22:51PM -0300, Alvaro Herrera wrote: On 2019-Sep-10, Binguo Bao wrote: +/* + * Support for de-TOASTing toasted value iteratively. "need" is a pointer + * between the beginning and end of iterator's ToastBuffer. The marco + * de-TOAST all bytes before "need" into itera

Re: [proposal] de-TOAST'ing using a iterator

2019-09-16 Thread Alvaro Herrera
On 2019-Sep-10, Binguo Bao wrote: > +/* > + * Support for de-TOASTing toasted value iteratively. "need" is a pointer > + * between the beginning and end of iterator's ToastBuffer. The marco > + * de-TOAST all bytes before "need" into iterator's ToastBuffer. > + */ > +#define PG_DETOAST_ITERATE(ite

Re: [proposal] de-TOAST'ing using a iterator

2019-09-10 Thread Binguo Bao
Alvaro Herrera 于2019年9月4日周三 上午4:12写道: > > +static void > > +init_toast_buffer(ToastBuffer *buf, int32 size, bool compressed) > > +{ > > + buf->buf = (const char *) palloc0(size); > > This API is weird -- you always palloc the ToastBuffer first, then call > init_toast_bufer on it. Why not pal

Re: [proposal] de-TOAST'ing using a iterator

2019-09-06 Thread Alvaro Herrera from 2ndQuadrant
Also: this patch no longer applies. Please rebase. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [proposal] de-TOAST'ing using a iterator

2019-09-03 Thread Alvaro Herrera
> +static void > +init_toast_buffer(ToastBuffer *buf, int32 size, bool compressed) > +{ > + buf->buf = (const char *) palloc0(size); This API is weird -- you always palloc the ToastBuffer first, then call init_toast_bufer on it. Why not palloc the ToastBuffer struct in init_toast_buffer and r

Re: [proposal] de-TOAST'ing using a iterator

2019-08-21 Thread John Naylor
On Thu, Aug 22, 2019 at 12:10 AM Binguo Bao wrote: > [v9 patch] Thanks, looks good. I'm setting it to ready for committer. -- John Naylorhttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [proposal] de-TOAST'ing using a iterator

2019-08-21 Thread Binguo Bao
John Naylor 于2019年8月19日周一 下午12:55写道: > init_toast_buffer(): > > + * Note the constrain buf->position <= buf->limit may be broken > + * at initialization. Make sure that the constrain is satisfied > + * when consume chars. > > s/constrain/constraint/ (2 times) > s/consume/consuming/ > > Also, this

Re: [proposal] de-TOAST'ing using a iterator

2019-08-18 Thread John Naylor
On Fri, Aug 16, 2019 at 10:48 PM Binguo Bao wrote: > [v8 patch with cosmetic changes] Okay, looks good. I'll make a few style suggestions and corrections. In the course of looking at this again, I have a few other questions below as well. It looks like you already do this for the most part, but

Re: [proposal] de-TOAST'ing using a iterator

2019-08-17 Thread Binguo Bao
Hi John, > >> Also, I don't think > >> the new logic for the ctrl/c variables is an improvement: > >> > >> 1. iter->ctrlc is intialized with '8' (even in the uncompressed case, > >> which is confusing). Any time you initialize with something not 0 or > >> 1, it's a magic number, and here it's far

Re: [proposal] de-TOAST'ing using a iterator

2019-08-13 Thread John Naylor
On Sat, Aug 3, 2019 at 11:11 PM Binguo Bao wrote: > > John Naylor 于2019年8月2日周五 下午3:12写道: >> >> I like the use of a macro here. However, I think we can find a better >> location for the definition. See the header comment of fmgr.h: >> "Definitions for the Postgres function manager and function-cal

Re: [proposal] de-TOAST'ing using a iterator

2019-08-03 Thread Binguo Bao
John Naylor 于2019年8月2日周五 下午3:12写道: > On Tue, Jul 30, 2019 at 8:20 PM Binguo Bao wrote: > > > > John Naylor 于2019年7月29日周一 上午11:49写道: > >> > >> 1). For every needle comparison, text_position_next_internal() > >> calculates how much of the value is needed and passes that to > >> detoast_iterate(),

Re: [proposal] de-TOAST'ing using a iterator

2019-08-02 Thread John Naylor
On Tue, Jul 30, 2019 at 8:20 PM Binguo Bao wrote: > > John Naylor 于2019年7月29日周一 上午11:49写道: >> >> 1). For every needle comparison, text_position_next_internal() >> calculates how much of the value is needed and passes that to >> detoast_iterate(), which then calculates if it has to do something or

Re: [proposal] de-TOAST'ing using a iterator

2019-07-30 Thread Binguo Bao
John Naylor 于2019年7月29日周一 上午11:49写道: > On Thu, Jul 25, 2019 at 10:21 PM Binguo Bao wrote: > My goal for this stage of review was to understand more fully what the > code is doing, and make it as simple and clear as possible, starting > at the top level. In doing so, it looks like I found some ad

Re: [proposal] de-TOAST'ing using a iterator

2019-07-28 Thread John Naylor
On Thu, Jul 25, 2019 at 10:21 PM Binguo Bao wrote: > > Hi John! > Sorry for the late reply. It took me some time to fix a random bug. Don't worry, it's not late at all! :-) >> In the case where we don't know the slice size, how about the other >> aspect of my question above: Might it be simpler

Re: [proposal] de-TOAST'ing using a iterator

2019-07-25 Thread Binguo Bao
Hi John! Sorry for the late reply. It took me some time to fix a random bug. In the case where we don't know the slice size, how about the other > aspect of my question above: Might it be simpler and less overhead to > decompress entire chunks at a time? If so, I think it would be > enlightening t

Re: [proposal] de-TOAST'ing using a iterator

2019-07-17 Thread John Naylor
On Tue, Jul 16, 2019 at 9:14 PM Binguo Bao wrote: > In the compressed beginning case, the test result is different from yours > since the patch is ~1.75x faster > rather than no improvement. The interesting thing is that the patch if 4% > faster than master in the uncompressed end case. > I can'

Re: [proposal] de-TOAST'ing using a iterator

2019-07-16 Thread Binguo Bao
Hi, John First, I'd like to advocate for caution when using synthetic > benchmarks involving compression. Consider this test: > insert into detoast_c (a) > select > 'abc'|| > repeat( > (SELECT string_agg(md5(chr(i)), '') > FROM generate_series(1,127) i) > , 1) > ||'xyz'

Re: [proposal] de-TOAST'ing using a iterator

2019-07-15 Thread John Naylor
On Wed, Jun 19, 2019 at 8:51 PM Binguo Bao wrote: > [v4 patch] Hi Binguo, I can verify I get no warnings with the v4 patch. I've done some additional performance testing. First, to sum up your results: > insert into detoast_c (a) select > repeat('1234567890-=abcdefghijklmnopqrstuvwxyz', 10

Re: [proposal] de-TOAST'ing using a iterator

2019-07-11 Thread Binguo Bao
I have set the local build configuration to be the same as on the CI. This patch should be correct. Best regards, Binguo Bao Binguo Bao 于2019年7月11日周四 上午12:39写道: > This is the patch that fix warnings. > > Best Regards, > Binguo Bao > > Binguo Bao 于2019年7月10日周三 下午10:18写道: > >> Hi Thomas, >> I've

Re: [proposal] de-TOAST'ing using a iterator

2019-07-10 Thread Binguo Bao
This is the patch that fix warnings. Best Regards, Binguo Bao Binguo Bao 于2019年7月10日周三 下午10:18写道: > Hi Thomas, > I've fixed the warnings. > > Thomas Munro 于2019年7月5日周五 下午12:21写道: > >> On Thu, Jun 20, 2019 at 1:51 AM Binguo Bao wrote: >> > Hi hackers! >> > This proposal aims to provide the abi

Re: [proposal] de-TOAST'ing using a iterator

2019-07-10 Thread Binguo Bao
Hi Thomas, I've fixed the warnings. Thomas Munro 于2019年7月5日周五 下午12:21写道: > On Thu, Jun 20, 2019 at 1:51 AM Binguo Bao wrote: > > Hi hackers! > > This proposal aims to provide the ability to de-TOAST a fully TOAST'd > and compressed field using an iterator and then update the appropriate > parts

Re: [proposal] de-TOAST'ing using a iterator

2019-07-04 Thread Thomas Munro
On Thu, Jun 20, 2019 at 1:51 AM Binguo Bao wrote: > Hi hackers! > This proposal aims to provide the ability to de-TOAST a fully TOAST'd and > compressed field using an iterator and then update the appropriate parts of > the code to use the iterator where possible instead of de-TOAST'ing and > d