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 2d071b2523b2e9654b5a43b7a1bad6aeaf307c78..d9aaac9ac2791f4b61f01bbe0aab52f22b14a7a0 100644 (file)
--- 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 f157df3532dd8901b197f107d6c48864f07d8aee..2ff85396d015cd4068720a57d6e30765d5e8c449 100644 (file)
--- 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 eb37da71689400840dc7c0f13513a85a33e48dbb..9146f59435b62f4a4862dc79791b1aa313c87804 100644 (file)
--- 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 f3bfc7bcf5ae0d440d40a17ff950ff87939ea5dd..83d61f8c7e0ddcf508abdba6ff895c98956aa56f 100644 (file)
--- 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 ed266d5c88f139a71651d3d8092ec27e1ac16b7f..f1ed021be2d3f4d2724428e4432b26e3aca00a08 100644 (file)
--- 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}');