Re: [COMMITTERS] pgsql: Clean up jsonb code.

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

In response to

Browse pgsql-committers by date

  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.

Browse pgsql-hackers by date

  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