From: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Geoghegan <pg(at)heroku(dot)com> |
Cc: | Alexander Korotkov <aekorotkov(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Oleg Bartunov <obartunov(at)gmail(dot)com> |
Subject: | Re: [COMMITTERS] pgsql: Clean up jsonb code. |
Date: | 2014-05-12 07:30:02 |
Message-ID: | 5370787A.3060203@vmware.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers pgsql-hackers |
On 05/10/2014 01:32 AM, Tom Lane wrote:
> Peter Geoghegan <pg(at)heroku(dot)com> writes:
>> On Fri, May 9, 2014 at 2:54 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> However, what it looks to me like we've got here is a very bad
>>> reimplementation of StringInfo buffers. There is for example no
>>> integer-overflow checking here. Rather than try to bring this code
>>> up to speed, I think we should rip it out and use StringInfo.
>
>> Heikki did specifically consider StringInfo buffers and said they were
>> not best suited to the task at hand. At the time I thought he meant
>> that he'd do something domain-specific to avoid unnecessary geometric
>> growth in the size of the buffer (I like to grow buffers to either
>> twice their previous size, or just big enough to fit the next thing,
>> whichever is larger), but that doesn't appear to be the case. Still,
>> it would be good to know what he meant before proceeding. It probably
>> had something to do with alignment.
>
> It looks to me like he wanted an API that would let him reserve space
> separately from filling it, which is not in stringinfo.c but is surely
> easily built on top of it.
Right, the API to reserve space separately was what I had in mind.
> For the moment, I've just gotten rid of
> the buggy code fragment in favor of calling enlargeStringInfo, which
> I trust to be right.
Thanks. I admit it didn't even occur to me to keep the localized API in
jsonb_utils as wrappers around appendString* functions. I only
considered two options: using appendString* directly, or doing
repalloc's in jsonb_utils.c. I like what you did there.
- Heikki
From | Date | Subject | |
---|---|---|---|
Next Message | Heikki Linnakangas | 2014-05-12 08:08:50 | pgsql: Free PQresult on error in pg_receivexlog. |
Previous Message | Tom Lane | 2014-05-11 21:17:42 | pgsql: Stamp 9.4beta1. |
From | Date | Subject | |
---|---|---|---|
Next Message | Heikki Linnakangas | 2014-05-12 07:52:59 | Re: Archive recovery won't be completed on some situation. |
Previous Message | Andres Freund | 2014-05-12 07:15:02 | Re: pg_class.relpages/allvisible probably shouldn't be a int4 |