jsonb failed assertions
Hello
I created relative large (about 200K lines) simple table (one jsonb column):
It works, but I got a errors:
TRAP: FailedAssertion("!(va.type == jbvArray || va.type == jbvObject)",
File: "jsonb_util.c", Line: 208)
LOG: server process (PID 3851) was terminated by signal 6: Aborted
DETAIL: Failed process was running: autovacuum: ANALYZE public.t4
LOG: terminating any other active server processes
WARNING: terminating connection because of crash of another server process
DETAIL: The postmaster has commanded this server process to roll back the
current transaction and exit, because ano
HINT: In a moment you should be able to reconnect to the database and
repeat your command.
WARNING: terminating connection because of crash of another server process
DETAIL: The postmaster has commanded this server process to roll back the
current transaction and exit, because ano
HINT: In a moment you should be able to reconnect to the database and
repeat your command.
LOG: all server processes terminated; reinitializing
LOG: database system was interrupted; last known up at 2014-05-20 20:07:32
CEST
LOG: database system was not properly shut down; automatic recovery in
progress
Data are based on opened citylots.json (avaiable on net by google)
postgres=# \lo_import ~/citylots.json
lo_import 16510
CREATE OR REPLACE FUNCTION public.bytea_to_text(bytea)
RETURNS text
LANGUAGE sql
AS $function$
SELECT convert_from($1, current_setting('server_encoding'))
$function$
postgres=# insert into xx select bytea_to_text(lo_get(16510));
INSERT 0 1
create table t1 (data json);
insert into t1 select a::json from xx;
postgres=# create table t3(data json);
CREATE TABLE
postgres=# insert into t3 select
json_array_elements(json_object_field(data, 'features')) from t1;
INSERT 0 206560
postgres=# insert into t4 select data::jsonb from t3;
INSERT 0 206560
postgres=# select * from t4 limit 1;
data
-----------------------------------------------------------------------------------------------------------------------
{"type": "Feature", "geometry": {"type": "Polygon", "coordinates":
[[[-122.422003528252475, 37.808480096967251, 0.0],.
. [-122.422076013325281, 37.808835019815085, 0.0], [-122.421102174348633,
37.808803534992904, 0.0], [-122.421062569067.
.274, 37.808601056818148, 0.0], [-122.422003528252475, 37.808480096967251,
0.0]]]}, "properties": {"TO_ST": "0", "BLKL.
.OT": "0001001", "STREET": "UNKNOWN", "FROM_ST": "0", "LOT_NUM": "001",
"ST_TYPE": null, "ODD_EVEN": "E", "BLOCK_NUM":.
. "0001", "MAPBLKLOT": "0001001"}}
(1 row)
postgres=# analyze t4;
The connection to the server was lost. Attempting reset: Failed
postgres=# select version();
version
-----------------------------------------------------------------------------------------------------------------
PostgreSQL 9.4beta1 on x86_64-unknown-linux-gnu, compiled by gcc (GCC)
4.8.2 20131212 (Red Hat 4.8.2-7), 64-bit
(1 row)
Program received signal SIGABRT, Aborted.
0x000000382fc35c39 in raise () from /lib64/libc.so.6
(gdb) bt
#0 0x000000382fc35c39 in raise () from /lib64/libc.so.6
#1 0x000000382fc37348 in abort () from /lib64/libc.so.6
#2 0x000000000078d8d7 in ExceptionalCondition (
conditionName=conditionName@entry=0x91daa0 "!(va.type == jbvArray ||
va.type == jbvObject)",
errorType=errorType@entry=0x7c62bc "FailedAssertion",
fileName=fileName@entry=0x91d5db "jsonb_util.c",
lineNumber=lineNumber@entry=208) at assert.c:54
#3 0x0000000000708815 in compareJsonbContainers (a=a@entry=0x7fd8371b7cbc,
b=b@entry=0x11537b4)
at jsonb_util.c:208
#4 0x0000000000706cc3 in jsonb_cmp (fcinfo=0x11aad18) at jsonb_op.c:244
#5 0x00000000007b32f9 in comparison_shim (x=<optimized out>, y=<optimized
out>, ssup=<optimized out>)
at sortsupport.c:53
#6 0x0000000000555893 in ApplySortComparator (isNull1=0 '\000', isNull2=0
'\000', ssup=0x7fff7aff12d0,
datum2=<optimized out>, datum1=<optimized out>) at
../../../src/include/utils/sortsupport.h:143
#7 compare_scalars (a=<optimized out>, b=<optimized out>,
arg=0x7fff7aff12c0) at analyze.c:2784
#8 0x00000000007c4625 in qsort_arg (a=a@entry=0x7fd834d3e048, n=<optimized
out>, n@entry=27648, es=es@entry=16,
cmp=cmp@entry=0x555870 <compare_scalars>, arg=arg@entry=0x7fff7aff12c0)
at qsort_arg.c:156
#9 0x0000000000554c21 in compute_scalar_stats (stats=0x11488f8,
fetchfunc=<optimized out>, samplerows=30000,
totalrows=206560) at analyze.c:2366
#10 0x0000000000556e1e in do_analyze_rel (onerel=onerel@entry=0x7fd8375c8950,
acquirefunc=<optimized out>,
relpages=18392, inh=inh@entry=0 '\000', elevel=elevel@entry=13,
vacstmt=0x10e6bf8, vacstmt=0x10e6bf8)
at analyze.c:528
#11 0x0000000000557bc1 in analyze_rel (relid=relid@entry=22085,
vacstmt=vacstmt@entry=0x10e6bf8,
bstrategy=<optimized out>) at analyze.c:268
#12 0x00000000005a9bf4 in vacuum (vacstmt=vacstmt@entry=0x10e6bf8,
relid=relid@entry=0,
do_toast=do_toast@entry=1 '\001', bstrategy=<optimized out>,
bstrategy@entry=0x0,
for_wraparound=for_wraparound@entry=0 '\000',
isTopLevel=isTopLevel@entry=1 '\001') at vacuum.c:251
#13 0x00000000006b1b9a in standard_ProcessUtility (parsetree=0x10e6bf8,
queryString=<optimized out>,
context=<optimized out>, params=0x0, dest=<optimized out>,
completionTag=<optimized out>) at utility.c:645
#14 0x00000000006aec11 in PortalRunUtility (portal=portal@entry=0x11428a8,
utilityStmt=utilityStmt@entry=0x10e6bf8, isTopLevel=isTopLevel@entry=1
'\001', dest=dest@entry=0x10e6fa0,
completionTag=completionTag@entry=0x7fff7aff1b40 "") at pquery.c:1187
#15 0x00000000006af8b2 in PortalRunMulti (portal=portal@entry=0x11428a8,
isTopLevel=isTopLevel@entry=1 '\001',
dest=dest@entry=0x10e6fa0, altdest=altdest@entry=0x10e6fa0,
completionTag=completionTag@entry=0x7fff7aff1b40 "") at pquery.c:1318
#16 0x00000000006b0460 in PortalRun (portal=portal@entry=0x11428a8,
count=count@entry=9223372036854775807,
isTopLevel=isTopLevel@entry=1 '\001', dest=dest@entry=0x10e6fa0,
altdest=altdest@entry=0x10e6fa0,
completionTag=completionTag@entry=0x7fff7aff1b40 "") at pquery.c:816
#17 0x00000000006ae03b in exec_simple_query (query_string=0x10e6158
"analyze t4;") at postgres.c:1045
#18 PostgresMain (argc=<optimized out>, argv=argv@entry=0x1080df0,
dbname=0x1080c50 "postgres",
username=<optimized out>) at postgres.c:4004
#19 0x0000000000461307 in BackendRun (port=0x10a0060) at postmaster.c:4117
#20 BackendStartup (port=0x10a0060) at postmaster.c:3791
#21 ServerLoop () at postmaster.c:1570
#22 0x000000000065404e in PostmasterMain (argc=argc@entry=3,
argv=argv@entry=0x107f5c0)
at postmaster.c:1223
#23 0x0000000000461fce in main (argc=3, argv=0x107f5c0) at main.c:225
I can share this data if there is some request - it is about 50MB
Regards
Pavel
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2014-05-20 20:40 GMT+02:00 Peter Geoghegan <pg@heroku.com>:
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?
va.type is zero .. jbvNull
Regards
Pavel
Show quoted text
--
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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"}}<<<
Regards
Pavel
2014-05-20 21:23 GMT+02:00 Peter Geoghegan <pg@heroku.com>:
Show quoted text
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2014-05-20 21:45 GMT+02:00 Peter Geoghegan <pg@heroku.com>:
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?
This json is printed by JsonToCString
Datum
jsonb_cmp(PG_FUNCTION_ARGS)
{
Jsonb *jba = PG_GETARG_JSONB(0);
Jsonb *jbb = PG_GETARG_JSONB(1);
int res;
char *jba_str = JsonbToCString(NULL, &jba->root, VARSIZE(jba));
char *jbb_str = JsonbToCString(NULL, &jbb->root, VARSIZE(jbb));
elog(NOTICE, "a:>> %s<<<", jba_str);
elog(NOTICE, "b:>> %s<<<", jbb_str);
pfree(jba_str);
pfree(jbb_str);
res = compareJsonbContainers(&jba->root, &jbb->root);
PG_FREE_IF_COPY(jba, 0);
PG_FREE_IF_COPY(jbb, 1);
PG_RETURN_INT32(res);
}
postgres=# select * from t3 where data->'properties'->>'MAPBLKLOT' =
'1377060' and data->'properties'->>'LOT_NUM' = '060';;
data
-------------------------------------------------------------------------------------------------------------------------------------
{ "type": "Feature", "properties": { "MAPBLKLOT": "1377060", "BLKLOT":
"1377060", "BLOCK_NUM": "1377", "LOT_NUM": "060", "FROM_ST":.
. null, "TO_ST": null, "STREET": null, "ST_TYPE": null, "ODD_EVEN": null },
"geometry": { "type": "Polygon", "coordinates": [ [ [ -1.
.22.476849622729347, 37.784637268897804, 0.0 ], [ -122.47693599079787,
37.784633351359254, 0.0 ], [ -122.477005086381169, 37.7846302.
.17263818, 0.0 ], [ -122.477010255706205, 37.784701504178585, 0.0 ], [
-122.476590928382066, 37.784720524837788, 0.0 ], [ -122.47658.
.5758323125, 37.784649237923851, 0.0 ], [ -122.476849622729347,
37.784637268897804, 0.0 ] ] ] } }
(1 row)
postgres=# select * from t3 where data->'properties'->>'MAPBLKLOT' =
'1245061' and data->'properties'->>'LOT_NUM' = '063';;
data
-------------------------------------------------------------------------------------------------------------------------------------
{ "type": "Feature", "properties": { "MAPBLKLOT": "1245061", "BLKLOT":
"1245063", "BLOCK_NUM": "1245", "LOT_NUM": "063", "FROM_ST":.
. null, "TO_ST": null, "STREET": null, "ST_TYPE": null, "ODD_EVEN": null },
"geometry": null }
(1 row)
table dump is downloadable from http://pgsql.cz/data/data.dump.gz
Regards
Pavel
Show quoted text
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
Attachments:
assertion_fix.patchtext/x-patch; charset=US-ASCII; name=assertion_fix.patchDownload
*** a/src/backend/utils/adt/jsonb_util.c
--- b/src/backend/utils/adt/jsonb_util.c
*************** static void uniqueifyJsonbObject(JsonbVa
*** 62,74 ****
*
* There isn't a JsonbToJsonbValue(), because generally we find it more
* convenient to directly iterate through the Jsonb representation and only
! * really convert nested scalar values. formIterIsContainer() does this, so
! * that clients of the iteration code don't have to directly deal with the
! * binary representation (JsonbDeepContains() is a notable exception, although
! * all exceptions are internal to this module). In general, functions that
! * accept a JsonbValue argument are concerned with the manipulation of scalar
! * values, or simple containers of scalar values, where it would be
! * inconvenient to deal with a great amount of other state.
*/
Jsonb *
JsonbValueToJsonb(JsonbValue *val)
--- 62,74 ----
*
* There isn't a JsonbToJsonbValue(), because generally we find it more
* convenient to directly iterate through the Jsonb representation and only
! * really convert nested scalar values. JsonbIteratorNext() does this, so that
! * clients of the iteration code don't have to directly deal with the binary
! * representation (JsonbDeepContains() is a notable exception, although all
! * exceptions are internal to this module). In general, functions that accept
! * a JsonbValue argument are concerned with the manipulation of scalar values,
! * or simple containers of scalar values, where it would be inconvenient to
! * deal with a great amount of other state.
*/
Jsonb *
JsonbValueToJsonb(JsonbValue *val)
*************** compareJsonbContainers(JsonbContainer *a
*** 198,212 ****
*
* If the two values were the same container type, then there'd
* have been a chance to observe the variation in the number of
! * elements/pairs (when processing WJB_BEGIN_OBJECT, say). They
! * can't be scalar types either, because then they'd have to be
! * contained in containers already ruled unequal due to differing
! * numbers of pairs/elements, or already directly ruled unequal
! * with a call to the underlying type's comparator.
*/
Assert(va.type != vb.type);
! Assert(va.type == jbvArray || va.type == jbvObject);
! Assert(vb.type == jbvArray || vb.type == jbvObject);
/* Type-defined order */
res = (va.type > vb.type) ? 1 : -1;
}
--- 198,210 ----
*
* If the two values were the same container type, then there'd
* have been a chance to observe the variation in the number of
! * elements/pairs (when processing WJB_BEGIN_OBJECT, say). They're
! * either two heterogeneously-typed containers, or a container and
! * some scalar type.
*/
Assert(va.type != vb.type);
! Assert(va.type != jbvBinary);
! Assert(vb.type != jbvBinary);
/* Type-defined order */
res = (va.type > vb.type) ? 1 : -1;
}
2014-05-21 3:22 GMT+02:00 Peter Geoghegan <pg@heroku.com>:
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
It works
Thank you
Regards
Pavel
Show quoted text
--
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
Attachments:
assertion_endcont_fix.patchtext/x-patch; charset=US-ASCII; name=assertion_endcont_fix.patchDownload
*** a/src/backend/utils/adt/jsonb_util.c
--- b/src/backend/utils/adt/jsonb_util.c
*************** static void uniqueifyJsonbObject(JsonbVa
*** 62,74 ****
*
* There isn't a JsonbToJsonbValue(), because generally we find it more
* convenient to directly iterate through the Jsonb representation and only
! * really convert nested scalar values. formIterIsContainer() does this, so
! * that clients of the iteration code don't have to directly deal with the
! * binary representation (JsonbDeepContains() is a notable exception, although
! * all exceptions are internal to this module). In general, functions that
! * accept a JsonbValue argument are concerned with the manipulation of scalar
! * values, or simple containers of scalar values, where it would be
! * inconvenient to deal with a great amount of other state.
*/
Jsonb *
JsonbValueToJsonb(JsonbValue *val)
--- 62,74 ----
*
* There isn't a JsonbToJsonbValue(), because generally we find it more
* convenient to directly iterate through the Jsonb representation and only
! * really convert nested scalar values. JsonbIteratorNext() does this, so that
! * clients of the iteration code don't have to directly deal with the binary
! * representation (JsonbDeepContains() is a notable exception, although all
! * exceptions are internal to this module). In general, functions that accept
! * a JsonbValue argument are concerned with the manipulation of scalar values,
! * or simple containers of scalar values, where it would be inconvenient to
! * deal with a great amount of other state.
*/
Jsonb *
JsonbValueToJsonb(JsonbValue *val)
*************** compareJsonbContainers(JsonbContainer *a
*** 137,149 ****
ra = JsonbIteratorNext(&ita, &va, false);
rb = JsonbIteratorNext(&itb, &vb, false);
- /*
- * 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.
- */
if (ra == rb)
{
if (ra == WJB_DONE)
--- 137,142 ----
*************** compareJsonbContainers(JsonbContainer *a
*** 152,157 ****
--- 145,162 ----
break;
}
+ if (ra == WJB_END_ARRAY || ra == WJB_END_OBJECT)
+ {
+ /*
+ * There is no array or object to safely compare at this stage
+ * of processing. jbvArray/jbvObject values are only
+ * considered initially, during processing of WJB_BEGIN_ARRAY
+ * and WJB_BEGIN_OBJECT tokens. This doesn't matter, because a
+ * second comparison would be redundant.
+ */
+ continue;
+ }
+
if (va.type == vb.type)
{
switch (va.type)
*************** compareJsonbContainers(JsonbContainer *a
*** 194,212 ****
else
{
/*
! * It's safe to assume that the types differed.
*
* If the two values were the same container type, then there'd
* have been a chance to observe the variation in the number of
! * elements/pairs (when processing WJB_BEGIN_OBJECT, say). They
! * can't be scalar types either, because then they'd have to be
! * contained in containers already ruled unequal due to differing
! * numbers of pairs/elements, or already directly ruled unequal
! * with a call to the underlying type's comparator.
*/
Assert(va.type != vb.type);
! Assert(va.type == jbvArray || va.type == jbvObject);
! Assert(vb.type == jbvArray || vb.type == jbvObject);
/* Type-defined order */
res = (va.type > vb.type) ? 1 : -1;
}
--- 199,227 ----
else
{
/*
! * It's safe to assume that the types differed, and that the va and
! * vb values passed were set.
*
+ * We don't have to consider the WJB_END_ARRAY and WJB_END_OBJECT
+ * cases here, because in order to process those tokens we'd first
+ * have to process WJB_BEGIN_ARRAY or WJB_BEGIN_OBJECT, and that
+ * would be sufficient reason to end up here (with reliable values
+ * set for va and vb), which always results in finishing the
+ * comparison.
+ */
+ Assert(ra != WJB_END_ARRAY && ra != WJB_END_OBJECT);
+ Assert(rb != WJB_END_ARRAY && rb != WJB_END_OBJECT);
+
+ /*
* If the two values were the same container type, then there'd
* have been a chance to observe the variation in the number of
! * elements/pairs (when processing WJB_BEGIN_OBJECT, say). They're
! * either two heterogeneously-typed containers, or a container and
! * some scalar type.
*/
Assert(va.type != vb.type);
! Assert(va.type != jbvBinary);
! Assert(vb.type != jbvBinary);
/* Type-defined order */
res = (va.type > vb.type) ? 1 : -1;
}
*************** JsonbIteratorInit(JsonbContainer *contai
*** 630,636 ****
* It is our job to expand the jbvBinary representation without bothering them
* with it. However, clients should not take it upon themselves to touch array
* or Object element/pair buffers, since their element/pair pointers are
! * garbage.
*/
JsonbIteratorToken
JsonbIteratorNext(JsonbIterator **it, JsonbValue *val, bool skipNested)
--- 645,653 ----
* It is our job to expand the jbvBinary representation without bothering them
* with it. However, clients should not take it upon themselves to touch array
* or Object element/pair buffers, since their element/pair pointers are
! * garbage. Also, *val will not be set when returning WJB_END_ARRAY or
! * WJB_END_OBJECT, on the assumption that it's only useful to access values
! * when recursing in.
*/
JsonbIteratorToken
JsonbIteratorNext(JsonbIterator **it, JsonbValue *val, bool skipNested)
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers