Re: Jsonb: jbvBinary usage in the convertJsonbValue?

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Peter Geoghegan <pg(at)heroku(dot)com>, Teodor Sigaev <teodor(at)sigaev(dot)ru>
Subject: Re: Jsonb: jbvBinary usage in the convertJsonbValue?
Date: 2014-06-02 14:22:49
Message-ID: 538C88B9.9010806@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On 06/02/2014 10:02 AM, Robert Haas wrote:
> On Thu, May 29, 2014 at 7:34 AM, Dmitry Dolgov <9erthalion6(at)gmail(dot)com> wrote:
>> I'm little confused by the convertJsonbValue functon at jsonb_utils.c
>> Maybe I misunderstood something, so I need help =)
>>
>>>>> if (IsAJsonbScalar(val) || val->type == jbvBinary)
>>>>> convertJsonbScalar(buffer, header, val);
>> As I can see, the convertJsonbScalar function is used for scalar and binary
>> jsonb values. But this function doesn't handle the jbvBinary type.
> There definitely seems to be an oversight here of some kind. Either
> convertJsonbValue() shouldn't be calling convertJsonbScalar() with an
> object of type jbvBinary, or convertJsonbScalar() should know how to
> handle that case.
>

Yes, I've just been looking at that. I think this is probably a hangover
from when these routines were recast to some extent. Given that we're
not seeing any errors from it, I'd be inclined to remove the the "||
val->type == jbvBinary" part. One of the three call sites to
convertJsonbValue asserts that this can't be true, and doing so doesn't
result in a regression failure.

Peter and Teodor, comments?

cheers

andrew

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2014-06-02 14:56:00 Re: Allowing join removals for more join types
Previous Message Tom Lane 2014-06-02 14:21:55 Re: Re-create dependent views on ALTER TABLE ALTER COLUMN ... TYPE?