Re: jsonb iterator not fully initialized - Mailing list pgsql-hackers

From Dmitry Dolgov
Subject Re: jsonb iterator not fully initialized
Date
Msg-id CA+q6zcWS6GJgFL+SjJG0U47DnUrupM3b9wS61ABDFrrh+hwf-A@mail.gmail.com
Whole thread Raw
In response to Re: jsonb iterator not fully initialized  (Andrew Dunstan <andrew.dunstan@2ndquadrant.com>)
List pgsql-hackers
> On 26 May 2018 at 16:47, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote:
> On 05/26/2018 03:09 AM, Piotr Stefaniak wrote:
>> On 2018-05-26 02:02, Peter Eisentraut wrote:
>>>
>>> It can be fixed this way:
>>>
>>> --- a/src/backend/utils/adt/jsonb_util.c
>>> +++ b/src/backend/utils/adt/jsonb_util.c
>>> @@ -901,7 +901,7 @@ iteratorFromContainer(JsonbContainer *container,
>>> JsonbIterator *parent)
>>>    {
>>>       JsonbIterator *it;
>>>
>>> -   it = palloc(sizeof(JsonbIterator));
>>> +   it = palloc0(sizeof(JsonbIterator));
>>>       it->container = container;
>>>       it->parent = parent;
>>>       it->nElems = JsonContainerSize(container);
>>>
>>> It's probably not a problem in practice, since the isScalar business is
>>> apparently only used in the array case, but it's dubious to leave things
>>> uninitialized like this nonetheless.

Yes, sounds reasonable.

>> I've seen it earlier but couldn't decide what my proposed fix should
>> look like. One of the options I considered was:
>>
>> --- a/src/backend/utils/adt/jsonfuncs.c
>> +++ b/src/backend/utils/adt/jsonfuncs.c
>> @@ -5010,10 +5010,8 @@ transform_jsonb_string_values(Jsonb *jsonb, void
>> *action_state,
>>           JsonbIteratorToken type;
>>           JsonbParseState *st = NULL;
>>           text       *out;
>> -       bool            is_scalar = false;
>>
>>           it = JsonbIteratorInit(&jsonb->root);
>> -       is_scalar = it->isScalar;
>>
>>           while ((type = JsonbIteratorNext(&it, &v, false)) != WJB_DONE)
>>           {
>> @@ -5033,7 +5031,7 @@ transform_jsonb_string_values(Jsonb *jsonb, void
>> *action_state,
>>           }
>>
>>           if (res->type == jbvArray)
>> -               res->val.array.rawScalar = is_scalar;
>> +               res->val.array.rawScalar =
>> JsonContainerIsScalar(&jsonb->root);
>>
>>           return JsonbValueToJsonb(res);
>>    }
>
> palloc0 seems cleaner.

Totally agree, palloc0 looks better (although I assume it's going to be
negligibly slower in those cases that aren't affected by this problem).


pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: found xmin from before relfrozenxid on pg_catalog.pg_authid
Next
From: Peter Geoghegan
Date:
Subject: Re: SP-GiST failing to complete SP-GiST index build