From: Tom Lane Date: Sat, 11 Oct 2014 18:13:51 +0000 (-0400) Subject: Fix bogus optimization in JSONB containment tests. X-Git-Tag: REL9_5_ALPHA1~1378 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4a50de13127b7657f32f14dc17beb2e15a3a4777;p=postgresql Fix bogus optimization in JSONB containment tests. When determining whether one JSONB object contains another, it's okay to make a quick exit if the first object has fewer pairs than the second: because we de-duplicate keys within objects, it is impossible that the first object has all the keys the second does. However, the code was applying this rule to JSONB arrays as well, where it does *not* hold because arrays can contain duplicate entries. The test was really in the wrong place anyway; we should do it within JsonbDeepContains, where it can be applied to nested objects not only top-level ones. Report and test cases by Alexander Korotkov; fix by Peter Geoghegan and Tom Lane. --- diff --git a/src/backend/utils/adt/jsonb_op.c b/src/backend/utils/adt/jsonb_op.c index 2d071b2523..d9aaac9ac2 100644 --- a/src/backend/utils/adt/jsonb_op.c +++ b/src/backend/utils/adt/jsonb_op.c @@ -57,7 +57,7 @@ jsonb_exists_any(PG_FUNCTION_ARGS) for (i = 0; i < elem_count; i++) { - JsonbValue strVal; + JsonbValue strVal; if (key_nulls[i]) continue; @@ -90,7 +90,7 @@ jsonb_exists_all(PG_FUNCTION_ARGS) for (i = 0; i < elem_count; i++) { - JsonbValue strVal; + JsonbValue strVal; if (key_nulls[i]) continue; @@ -117,8 +117,7 @@ jsonb_contains(PG_FUNCTION_ARGS) JsonbIterator *it1, *it2; - if (JB_ROOT_COUNT(val) < JB_ROOT_COUNT(tmpl) || - JB_ROOT_IS_OBJECT(val) != JB_ROOT_IS_OBJECT(tmpl)) + if (JB_ROOT_IS_OBJECT(val) != JB_ROOT_IS_OBJECT(tmpl)) PG_RETURN_BOOL(false); it1 = JsonbIteratorInit(&val->root); @@ -137,8 +136,7 @@ jsonb_contained(PG_FUNCTION_ARGS) JsonbIterator *it1, *it2; - if (JB_ROOT_COUNT(val) < JB_ROOT_COUNT(tmpl) || - JB_ROOT_IS_OBJECT(val) != JB_ROOT_IS_OBJECT(tmpl)) + if (JB_ROOT_IS_OBJECT(val) != JB_ROOT_IS_OBJECT(tmpl)) PG_RETURN_BOOL(false); it1 = JsonbIteratorInit(&val->root); diff --git a/src/backend/utils/adt/jsonb_util.c b/src/backend/utils/adt/jsonb_util.c index f157df3532..2ff85396d0 100644 --- a/src/backend/utils/adt/jsonb_util.c +++ b/src/backend/utils/adt/jsonb_util.c @@ -957,13 +957,24 @@ JsonbDeepContains(JsonbIterator **val, JsonbIterator **mContained) } else if (rcont == WJB_BEGIN_OBJECT) { - JsonbValue *lhsVal; /* lhsVal is from pair in lhs object */ - + Assert(vval.type == jbvObject); Assert(vcontained.type == jbvObject); + /* + * If the lhs has fewer pairs than the rhs, it can't possibly contain + * the rhs. (This conclusion is safe only because we de-duplicate + * keys in all Jsonb objects; thus there can be no corresponding + * optimization in the array case.) The case probably won't arise + * often, but since it's such a cheap check we may as well make it. + */ + if (vval.val.object.nPairs < vcontained.val.object.nPairs) + return false; + /* Work through rhs "is it contained within?" object */ for (;;) { + JsonbValue *lhsVal; /* lhsVal is from pair in lhs object */ + rcont = JsonbIteratorNext(mContained, &vcontained, false); /* @@ -1047,6 +1058,7 @@ JsonbDeepContains(JsonbIterator **val, JsonbIterator **mContained) JsonbValue *lhsConts = NULL; uint32 nLhsElems = vval.val.array.nElems; + Assert(vval.type == jbvArray); Assert(vcontained.type == jbvArray); /* diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out index eb37da7168..9146f59435 100644 --- a/src/test/regress/expected/jsonb.out +++ b/src/test/regress/expected/jsonb.out @@ -707,6 +707,42 @@ SELECT '{"a":"b", "b":1, "c":null}'::jsonb @> '{"a":"b", "c":"q"}'; f (1 row) +SELECT '[1,2]'::jsonb @> '[1,2,2]'::jsonb; + ?column? +---------- + t +(1 row) + +SELECT '[1,1,2]'::jsonb @> '[1,2,2]'::jsonb; + ?column? +---------- + t +(1 row) + +SELECT '[[1,2]]'::jsonb @> '[[1,2,2]]'::jsonb; + ?column? +---------- + t +(1 row) + +SELECT '[1,2,2]'::jsonb <@ '[1,2]'::jsonb; + ?column? +---------- + t +(1 row) + +SELECT '[1,2,2]'::jsonb <@ '[1,1,2]'::jsonb; + ?column? +---------- + t +(1 row) + +SELECT '[[1,2,2]]'::jsonb <@ '[[1,2]]'::jsonb; + ?column? +---------- + t +(1 row) + SELECT jsonb_contained('{"a":"b"}', '{"a":"b", "b":1, "c":null}'); jsonb_contained ----------------- diff --git a/src/test/regress/expected/jsonb_1.out b/src/test/regress/expected/jsonb_1.out index f3bfc7bcf5..83d61f8c7e 100644 --- a/src/test/regress/expected/jsonb_1.out +++ b/src/test/regress/expected/jsonb_1.out @@ -707,6 +707,42 @@ SELECT '{"a":"b", "b":1, "c":null}'::jsonb @> '{"a":"b", "c":"q"}'; f (1 row) +SELECT '[1,2]'::jsonb @> '[1,2,2]'::jsonb; + ?column? +---------- + t +(1 row) + +SELECT '[1,1,2]'::jsonb @> '[1,2,2]'::jsonb; + ?column? +---------- + t +(1 row) + +SELECT '[[1,2]]'::jsonb @> '[[1,2,2]]'::jsonb; + ?column? +---------- + t +(1 row) + +SELECT '[1,2,2]'::jsonb <@ '[1,2]'::jsonb; + ?column? +---------- + t +(1 row) + +SELECT '[1,2,2]'::jsonb <@ '[1,1,2]'::jsonb; + ?column? +---------- + t +(1 row) + +SELECT '[[1,2,2]]'::jsonb <@ '[[1,2]]'::jsonb; + ?column? +---------- + t +(1 row) + SELECT jsonb_contained('{"a":"b"}', '{"a":"b", "b":1, "c":null}'); jsonb_contained ----------------- diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql index ed266d5c88..f1ed021be2 100644 --- a/src/test/regress/sql/jsonb.sql +++ b/src/test/regress/sql/jsonb.sql @@ -156,6 +156,13 @@ SELECT '{"a":"b", "b":1, "c":null}'::jsonb @> '{"a":"c"}'; SELECT '{"a":"b", "b":1, "c":null}'::jsonb @> '{"a":"b"}'; SELECT '{"a":"b", "b":1, "c":null}'::jsonb @> '{"a":"b", "c":"q"}'; +SELECT '[1,2]'::jsonb @> '[1,2,2]'::jsonb; +SELECT '[1,1,2]'::jsonb @> '[1,2,2]'::jsonb; +SELECT '[[1,2]]'::jsonb @> '[[1,2,2]]'::jsonb; +SELECT '[1,2,2]'::jsonb <@ '[1,2]'::jsonb; +SELECT '[1,2,2]'::jsonb <@ '[1,1,2]'::jsonb; +SELECT '[[1,2,2]]'::jsonb <@ '[[1,2]]'::jsonb; + SELECT jsonb_contained('{"a":"b"}', '{"a":"b", "b":1, "c":null}'); SELECT jsonb_contained('{"a":"b", "c":null}', '{"a":"b", "b":1, "c":null}'); SELECT jsonb_contained('{"a":"b", "g":null}', '{"a":"b", "b":1, "c":null}');