On 05/18/2015 08:17 AM, Andrew Dunstan wrote:
>
> On 05/17/2015 09:08 PM, Peter Geoghegan wrote:
>> On Sun, May 17, 2015 at 8:04 AM, Andrew Dunstan <andrew@dunslane.net>
>> wrote:
>>> Sure. I thought we'd covered this but it's possible that we didn't,
>>> or that
>>> it got rebroken. There have been complaints about the limitation on
>>> values
>>> containing jbvBinary, so let's just remove it if that can be done
>>> simply, as
>>> it seems to be a not uncommonly encountered problem.
>>>
>>> Are you going to submit a patch for that?
>> I'll try and come up with something. It's not a trivial fix.
>
>
>
> Here's a thought. in pushJsonbValue() the value pushed is called
> scalarVal, and in all our regression tests it is in fact scalar.
> However, the Asserts inside the function tell a different story:
>
> case WJB_VALUE:
> Assert(IsAJsonbScalar(scalarVal) ||
> scalarVal->type == jbvBinary);
> appendValue(*pstate, scalarVal);
> break;
> case WJB_ELEM:
> Assert(IsAJsonbScalar(scalarVal) ||
> scalarVal->type == jbvBinary);
> appendElement(*pstate, scalarVal);
> break;
>
> and indeed it turns out that your delete example does push a value
> with jbvBinary, thus triggering the problem. So, could we deal with
> that, by, say, setting up an iterator for the binary datum and just
> pushing all of its values?
>
>
Here's an patch along those lines. It seems to do the trick, at least
for your test case, and it has the merit of being very small, so small
I'd like to backpatch it - accepting jbvBinary as is in pushJsonbValue
seems like a bug to me.
cheers
andrew