Обсуждение: jsonb failed assertions

On Tue, May 20, 2014 at 11:23 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> TRAP: FailedAssertion("!(va.type == jbvArray || va.type == jbvObject)",
> File: "jsonb_util.c", Line: 208)

So, what type is va.type, if not one of those two?

-- 
Peter Geoghegan

On Tue, May 20, 2014 at 11:40 AM, Peter Geoghegan <pg@heroku.com> wrote:
>
> So, what type is va.type, if not one of those two?

You should be able to figure out which pair of JSON documents are
being compared by printing JsonbIterator.dataProper within GDB (that
is, you should do so for both ita and itb within
compareJsonbContainers()). That might allow you to produce a simple
test-case.

I am in the airport right now, about to fly to pgCon, and so may not
have a chance to look at this properly for a day or two.

Thanks.
-- 
Peter Geoghegan

On Tue, May 20, 2014 at 12:03 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> va.type is zero .. jbvNull


Thanks...any luck with identifying the two JSON documents?

-- 
Peter Geoghegan

On Tue, May 20, 2014 at 12:38 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> NOTICE:  a:>> {"type": "Feature", "geometry": null, "properties": {"TO_ST":
> null, "BLKLOT": "1245063", "STREET": null, "FROM_ST": null, "LOT_NUM":
> "063", "ST_TYPE": null, "ODD_EVEN": null, "BLOCK_NUM": "1245", "MAPBLKLOT":
> "1245061"}}<<<
> NOTICE:  b:>> {"type": "Feature", "geometry": {"type": "Polygon",
> "coordinates": [[[-122.476849622729347, 37.784637268897804, 0.0],
> [-122.47693599079787, 37.784633351359254, 0.0], [-122.477005086381169,
> 37.784630217263818, 0.0], [-122.477010255706205, 37.784701504178585, 0.0],
> [-122.476590928382066, 37.784720524837788, 0.0], [-122.476585758323125,
> 37.784649237923851, 0.0], [-122.476849622729347, 37.784637268897804,
> 0.0]]]}, "properties": {"TO_ST": null, "BLKLOT": "1377060", "STREET": null,
> "FROM_ST": null, "LOT_NUM": "060", "ST_TYPE": null, "ODD_EVEN": null,
> "BLOCK_NUM": "1377", "MAPBLKLOT": "1377060"}}<<<

I cannot immediately reproduce the problem.

Is this the original JSON? Can you show the psql output from a query
with a predicate that returns both jsonb datums?

Thanks
-- 
Peter Geoghegan

On Tue, May 20, 2014 at 4:17 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> table dump is downloadable from http://pgsql.cz/data/data.dump.gz

This looks like an over-zealous assertion, without any user-visible
consequences. Mea culpa.

Attached patch corrects the problem.  I also noticed in passing that
there is another obsolete comment -- formIterIsContainer() is former
jsonbIteratorNext() infrastructure, which the updated comment now
refers to directly.

Thanks
--
Peter Geoghegan

On 05/20/2014 09:22 PM, Peter Geoghegan wrote:
> On Tue, May 20, 2014 at 4:17 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> table dump is downloadable from http://pgsql.cz/data/data.dump.gz
>
> This looks like an over-zealous assertion, without any user-visible
> consequences. Mea culpa.
>
> Attached patch corrects the problem.  I also noticed in passing that
> there is another obsolete comment -- formIterIsContainer() is former
> jsonbIteratorNext() infrastructure, which the updated comment now
> refers to directly.

Hmm. The patch looks correct as far as it goes. But that function is 
still a bit funny. When it compares two identical arrays (or objects), 
and reaches the WJB_END_ARRAY token, it will still fall into the code 
that checks what the va and vb types are, and compares the last scalar 
values in that array again. That's wrong, and will fail if the compiler 
decides to clobber the local "va" or "vb" variables between iterations 
of the do-while loop, because JsonbIteratorNext() does not set the value 
when returning WJB_END_ARRAY.

BTW, I don't understand this comment:

>         /*
>          * To a limited extent we'll redundantly iterate over an array/object
>          * while re-performing the same test without any reasonable
>          * expectation of the same container types having differing lengths
>          * (as when we process a WJB_BEGIN_OBJECT, and later the corresponding
>          * WJB_END_OBJECT), but no matter.
>          */

Can you elaborate?

- Heikki

On Wed, May 21, 2014 at 4:53 AM, Heikki Linnakangas
<hlinnakangas@vmware.com> wrote:
> Hmm. The patch looks correct as far as it goes. But that function is still a
> bit funny. When it compares two identical arrays (or objects), and reaches
> the WJB_END_ARRAY token, it will still fall into the code that checks what
> the va and vb types are, and compares the last scalar values in that array
> again. That's wrong, and will fail if the compiler decides to clobber the
> local "va" or "vb" variables between iterations of the do-while loop,
> because JsonbIteratorNext() does not set the value when returning
> WJB_END_ARRAY.

That's an obsolete assumption that once actually applied during development.

Attached revision also adds handling for that case.

Thanks
--
Peter Geoghegan

On 05/28/2014 04:13 AM, Peter Geoghegan wrote:
> On Wed, May 21, 2014 at 4:53 AM, Heikki Linnakangas
> <hlinnakangas@vmware.com> wrote:
>> Hmm. The patch looks correct as far as it goes. But that function is still a
>> bit funny. When it compares two identical arrays (or objects), and reaches
>> the WJB_END_ARRAY token, it will still fall into the code that checks what
>> the va and vb types are, and compares the last scalar values in that array
>> again. That's wrong, and will fail if the compiler decides to clobber the
>> local "va" or "vb" variables between iterations of the do-while loop,
>> because JsonbIteratorNext() does not set the value when returning
>> WJB_END_ARRAY.
>
> That's an obsolete assumption that once actually applied during development.
>
> Attached revision also adds handling for that case.

Thanks, applied.

- Heikki