On 05/18/2015 10:52 PM, Peter Geoghegan wrote:
On Mon, May 18, 2015 at 7:11 AM, Andrew Dunstan <and...@dunslane.net> wrote:
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.
Isn't that for the benefit of raw scalar pseudo arrays? The existing
comments above pushJsonbValue() acknowledge such callers.


Umm, no, the raw scalar pseudo arrays are of type jbvArray, not jbvBinary. And they are pushed with WJB_BEGIN_ARRAY, not with WJB_ELEM or WJB_VALUE, as the comment notes. See this fragment of code from JsonbValueToJsonb:

        scalarArray.type = jbvArray;
        scalarArray.val.array.rawScalar = true;
        scalarArray.val.array.nElems = 1;

        pushJsonbValue(&pstate, WJB_BEGIN_ARRAY, &scalarArray);


I tested this by removing the assert test for jbvBinary in the WJB_ELEM and WJB_VALUE switch beranches and then running the regression suite. No assertion failure was triggered. While that's not guaranteed to be a perfect test, it doesn't seem like a bad one. Can you pose a counter example where this will break?



Why are you passing the skipNested variable to JsonbIteratorNext()
within jsonb_delete()? I'm not seeing a need for that.


If you don't the logic gets more complex, as you need to keep track of what level of the object you are at. The virtue of this change is that it will simplify a lot of such processing by removing the unnecessary restriction on passing jbvBinary values to pushJsonbValue().

If you have a better fix for the bug you complained about I'll be happy to take a look.

cheers

andrew






--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to