Обсуждение: Optimization of some jsonb functions

Поиск
Список
Период
Сортировка

Optimization of some jsonb functions

От
Nikita Glukhov
Дата:
Attached set of patches with some jsonb optimizations that were made during
comparison of performance of ordinal jsonb operators and jsonpath operators.

1. Optimize JsonbExtractScalar():  It is better to use getIthJsonbValueFromContainer(cont, 0) instead of  JsonIterator to get 0th element of raw-scalar pseudoarray.  JsonbExtractScalar() is used in jsonb casts, so they speed up a bit.

2. Optimize operator #>>, jsonb_each_text(), jsonb_array_elements_text():  These functions have direct conversion (JsonbValue => text) only for  jbvString scalars, but indirect conversion of other types of scalars   (JsonbValue => jsonb => text) is obviously too slow.  Extracted common  subroutine JsonbValueAsText() and used in all suitable places.

3. Optimize JsonbContainer type recognition in get_jsonb_path_all():  Fetching of the first token from JsonbIterator is replaced with lightweight  JsonbContainerIsXxx() macros.

4. Extract findJsonbKeyInObject():  Extracted findJsonbKeyInObject() from findJsonbValueFromContainer(),  which is slightly easier to use (key string and its length is passed instead  of filled string JsonbValue).

5. Optimize resulting value allocation in findJsonbValueFromContainer() and  getIthJsonbValueFromContainer():  Added ability to pass stack-allocated JsonbValue that will be filled with  the result of operation instead of returning unconditionally palloc()ated  JsonbValue.

Patches #4 and #5 are mostly refactorings, but they can give small speedup
(up to 5% for upcoming jsonpath operators) due to elimination of unnecessary 
palloc()s.  The whole interface of findJsonbValueFromContainer() with JB_OBJECT
and JB_ARRAY flags always seemed a bit strange to me, so I think it is worth to
have separate functions for searching keys in objects and elements in arrays.


Performance tests:- Test data for {"x": {"y": {"z": i}}}:  CREATE TABLE t AS  SELECT jsonb_build_object('x',           jsonb_build_object('y',             jsonb_build_object('z', i))) js  FROM generate_series(1, 3000000) i;
- Sample query:  EXPLAIN (ANALYZE) SELECT js -> 'x' -> 'y' -> 'z' FROM t;
- Results:                                                   |   execution time, ms                        query                      |  master  |   optimized   
-------------------------------------------------------------------------------             {"x": {"y": {"z": i}}}js #> '{x,y,z}'                                    | 1148.632 | 1005.578 -10%js #>> '{x,y,z}'                                   | 1520.160 |  849.991 -40%(js #> '{x,y,z}')::numeric                         | 1310.881 | 1067.752 -20%(js #>> '{x,y,z}')::numeric                        | 1757.179 | 1109.495 -30%
js -> 'x' -> 'y' -> 'z'                            | 1030.211 |  977.267js -> 'x' -> 'y' ->> 'z'                           |  887.101 |  838.745(js -> 'x' -> 'y' -> 'z')::numeric                 | 1184.086 | 1050.462(js -> 'x' -> 'y' -> 'z')::int4                    | 1279.315 | 1133.032(js -> 'x' -> 'y' ->> 'z')::numeric                | 1134.003 | 1100.047(js -> 'x' -> 'y' ->> 'z')::int4                   | 1077.216 |  991.995
js ? 'x'                                           |  523.111 |  495.387js ?| '{x,y,z}'                                    |  612.880 |  607.455js ?& '{x,y,z}'                                    |  674.786 |  643.987js -> 'x' -> 'y' ? 'z'                             |  712.623 |  698.588js @> '{"x": {"y": {"z": 1}}}'                     | 1154.926 | 1149.069

jsonpath:js @@ '$.x.y.z == 123'                             |  973,444 |   912,08  -5%
             {"x": i, "y": i, "z": i}jsonb_each(js)                                     | 2281.577 | 2262.660jsonb_each_text(js)                                | 2603.539 | 2112.200 -20%                  [i, i, i]jsonb_array_elements(js)                           | 1255.210 | 1205.939jsonb_array_elements(js)::numeric                  | 1662.550 | 1576.227  -5%jsonb_array_elements_text(js)                      | 1555.021 | 1067.031 -30%
js @> '1'                                          |  798.858 |  768.664  -4%js <@ '[1,2,3]'                                    |  820.795 |  785.086  -5%js <@ '[0,1,2,3,4,5,6,7,8,9]'                      | 1214.170 | 1165.289  -5%


As it can be seen, #> operators are always slower than equivalent series of ->.
I think it is caused by array deconstruction in "jsonb #> text[]".

-- 
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Вложения

Re: Optimization of some jsonb functions

От
David Steele
Дата:
On 2/22/19 2:05 AM, Nikita Glukhov wrote:
> Attached set of patches with some jsonb optimizations that were made during
> comparison of performance of ordinal jsonb operators and jsonpath operators.

This patch was submitted just before the last commitfest for PG12 and 
seems to have potential for breakage.

I have updated the target to PG13.

Regards,
-- 
-David
david@pgmasters.net


Re: Optimization of some jsonb functions

От
Andrew Dunstan
Дата:
On 3/5/19 5:24 AM, David Steele wrote:
> On 2/22/19 2:05 AM, Nikita Glukhov wrote:
>> Attached set of patches with some jsonb optimizations that were made
>> during
>> comparison of performance of ordinal jsonb operators and jsonpath
>> operators.
>
> This patch was submitted just before the last commitfest for PG12 and
> seems to have potential for breakage.
>
> I have updated the target to PG13.
>
>

I think that's overly cautious. The first one I looked at, to optimize
JsonbExtractScalar, is very small, self-contained, and I think low risk.
I haven't looked at the others in detail, but I think at least some part
of this is reasonably committable.


I'll try to look at the others fairly shortly.


cheers


andrew



-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: Optimization of some jsonb functions

От
David Steele
Дата:
Hi Andrew,

On 3/6/19 9:50 PM, Andrew Dunstan wrote:
> 
> On 3/5/19 5:24 AM, David Steele wrote:
>> On 2/22/19 2:05 AM, Nikita Glukhov wrote:
>>> Attached set of patches with some jsonb optimizations that were made
>>> during
>>> comparison of performance of ordinal jsonb operators and jsonpath
>>> operators.
>>
>> This patch was submitted just before the last commitfest for PG12 and
>> seems to have potential for breakage.
>>
>> I have updated the target to PG13.
>>
>>
> 
> I think that's overly cautious. The first one I looked at, to optimize
> JsonbExtractScalar, is very small, self-contained, and I think low risk.
> I haven't looked at the others in detail, but I think at least some part
> of this is reasonably committable.
> 
> 
> I'll try to look at the others fairly shortly.

If you decide all or part of this can be committed then feel free to 
update the target version.

Regards,
-- 
-David
david@pgmasters.net


Re: Optimization of some jsonb functions

От
Thomas Munro
Дата:
> >> On 2/22/19 2:05 AM, Nikita Glukhov wrote:
> >>> Attached set of patches with some jsonb optimizations that were made
> >>> during
> >>> comparison of performance of ordinal jsonb operators and jsonpath
> >>> operators.

Hi Nikita,

This doesn't apply -- to attract reviewers, could we please have a rebase?

Thanks,

-- 
Thomas Munro
https://enterprisedb.com



Re: Optimization of some jsonb functions

От
Joe Nelson
Дата:
Thomas Munro wrote:
> This doesn't apply -- to attract reviewers, could we please have a rebase?

To help the review go forward, I have rebased the patch on 27cd521e6e.
It passes `make check` for me, but that's as far as I've verified the
correctness.

I squashed the changes into a single patch, sorry if that makes it
harder to review than the original set of five patch files...

--
Joe Nelson      https://begriffs.com
diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
index 69f41ab455..8dced4ef6c 100644
--- a/src/backend/utils/adt/jsonb.c
+++ b/src/backend/utils/adt/jsonb.c
@@ -1873,9 +1873,7 @@ jsonb_object_agg_finalfn(PG_FUNCTION_ARGS)
 bool
 JsonbExtractScalar(JsonbContainer *jbc, JsonbValue *res)
 {
-    JsonbIterator *it;
-    JsonbIteratorToken tok PG_USED_FOR_ASSERTS_ONLY;
-    JsonbValue    tmp;
+    JsonbValue  *scalar PG_USED_FOR_ASSERTS_ONLY;
 
     if (!JsonContainerIsArray(jbc) || !JsonContainerIsScalar(jbc))
     {
@@ -1884,25 +1882,8 @@ JsonbExtractScalar(JsonbContainer *jbc, JsonbValue *res)
         return false;
     }
 
-    /*
-     * A root scalar is stored as an array of one element, so we get the array
-     * and then its first (and only) member.
-     */
-    it = JsonbIteratorInit(jbc);
-
-    tok = JsonbIteratorNext(&it, &tmp, true);
-    Assert(tok == WJB_BEGIN_ARRAY);
-    Assert(tmp.val.array.nElems == 1 && tmp.val.array.rawScalar);
-
-    tok = JsonbIteratorNext(&it, res, true);
-    Assert(tok == WJB_ELEM);
-    Assert(IsAJsonbScalar(res));
-
-    tok = JsonbIteratorNext(&it, &tmp, true);
-    Assert(tok == WJB_END_ARRAY);
-
-    tok = JsonbIteratorNext(&it, &tmp, true);
-    Assert(tok == WJB_DONE);
+    scalar = getIthJsonbValueFromContainer(jbc, 0, res);
+    Assert(scalar);
 
     return true;
 }
diff --git a/src/backend/utils/adt/jsonb_op.c b/src/backend/utils/adt/jsonb_op.c
index a64206eeb1..82c4b0b2cb 100644
--- a/src/backend/utils/adt/jsonb_op.c
+++ b/src/backend/utils/adt/jsonb_op.c
@@ -24,6 +24,7 @@ jsonb_exists(PG_FUNCTION_ARGS)
     Jsonb       *jb = PG_GETARG_JSONB_P(0);
     text       *key = PG_GETARG_TEXT_PP(1);
     JsonbValue    kval;
+    JsonbValue    vval;
     JsonbValue *v = NULL;
 
     /*
@@ -38,7 +39,7 @@ jsonb_exists(PG_FUNCTION_ARGS)
 
     v = findJsonbValueFromContainer(&jb->root,
                                     JB_FOBJECT | JB_FARRAY,
-                                    &kval);
+                                    &kval, &vval);
 
     PG_RETURN_BOOL(v != NULL);
 }
@@ -59,6 +60,7 @@ jsonb_exists_any(PG_FUNCTION_ARGS)
     for (i = 0; i < elem_count; i++)
     {
         JsonbValue    strVal;
+        JsonbValue    valVal;
 
         if (key_nulls[i])
             continue;
@@ -69,7 +71,7 @@ jsonb_exists_any(PG_FUNCTION_ARGS)
 
         if (findJsonbValueFromContainer(&jb->root,
                                         JB_FOBJECT | JB_FARRAY,
-                                        &strVal) != NULL)
+                                        &strVal, &valVal) != NULL)
             PG_RETURN_BOOL(true);
     }
 
@@ -92,6 +94,7 @@ jsonb_exists_all(PG_FUNCTION_ARGS)
     for (i = 0; i < elem_count; i++)
     {
         JsonbValue    strVal;
+        JsonbValue    valVal;
 
         if (key_nulls[i])
             continue;
@@ -102,7 +105,7 @@ jsonb_exists_all(PG_FUNCTION_ARGS)
 
         if (findJsonbValueFromContainer(&jb->root,
                                         JB_FOBJECT | JB_FARRAY,
-                                        &strVal) == NULL)
+                                        &strVal, &valVal) == NULL)
             PG_RETURN_BOOL(false);
     }
 
diff --git a/src/backend/utils/adt/jsonb_util.c b/src/backend/utils/adt/jsonb_util.c
index ac04c4a57b..05e1c18472 100644
--- a/src/backend/utils/adt/jsonb_util.c
+++ b/src/backend/utils/adt/jsonb_util.c
@@ -57,6 +57,8 @@ static void appendValue(JsonbParseState *pstate, JsonbValue *scalarVal);
 static void appendElement(JsonbParseState *pstate, JsonbValue *scalarVal);
 static int    lengthCompareJsonbStringValue(const void *a, const void *b);
 static int    lengthCompareJsonbPair(const void *a, const void *b, void *arg);
+static int  lengthCompareJsonbString(const char *val1, int len1,
+                                     const char *val2, int len2);
 static void uniqueifyJsonbObject(JsonbValue *object);
 static JsonbValue *pushJsonbValueScalar(JsonbParseState **pstate,
                                         JsonbIteratorToken seq,
@@ -297,6 +299,102 @@ compareJsonbContainers(JsonbContainer *a, JsonbContainer *b)
     return res;
 }
 
+/* Find scalar element in Jsonb array and return it. */
+static JsonbValue *
+findJsonbElementInArray(JsonbContainer *container, JsonbValue *elem,
+                        JsonbValue *res)
+{
+    JsonbValue *result;
+    JEntry       *children = container->children;
+    int            count = JsonContainerSize(container);
+    char       *baseAddr = (char *) (children + count);
+    uint32        offset = 0;
+    int            i;
+
+    result = res ? res : palloc(sizeof(*result));
+
+    for (i = 0; i < count; i++)
+    {
+        fillJsonbValue(container, i, baseAddr, offset, result);
+
+        if (elem->type == result->type &&
+            equalsJsonbScalarValue(elem, result))
+            return result;
+
+        JBE_ADVANCE_OFFSET(offset, children[i]);
+    }
+
+    if (!res)
+        pfree(result);
+
+    return NULL;
+}
+
+/* Find value by key in Jsonb object and fetch it into 'res'. */
+JsonbValue *
+findJsonbKeyInObject(JsonbContainer *container, const char *keyVal, int keyLen,
+                     JsonbValue *res)
+{
+    JEntry       *children = container->children;
+    JsonbValue *result = res;
+    int            count = JsonContainerSize(container);
+    /* Since this is an object, account for *Pairs* of Jentrys */
+    char       *baseAddr = (char *) (children + count * 2);
+    uint32        stopLow = 0,
+                stopHigh = count;
+
+    Assert(JsonContainerIsObject(container));
+
+    /* Quick out without a palloc cycle if object is empty */
+    if (count <= 0)
+        return NULL;
+
+    if (!result)
+        result = palloc(sizeof(JsonbValue));
+
+    /* Binary search on object/pair keys *only* */
+    while (stopLow < stopHigh)
+    {
+        uint32        stopMiddle;
+        int            difference;
+        const char *candidateVal;
+        int            candidateLen;
+
+        stopMiddle = stopLow + (stopHigh - stopLow) / 2;
+
+        candidateVal = baseAddr + getJsonbOffset(container, stopMiddle);
+        candidateLen = getJsonbLength(container, stopMiddle);
+
+        difference = lengthCompareJsonbString(candidateVal, candidateLen,
+                                              keyVal, keyLen);
+
+        if (difference == 0)
+        {
+            /* Found our key, return corresponding value */
+            int            index = stopMiddle + count;
+
+            fillJsonbValue(container, index, baseAddr,
+                           getJsonbOffset(container, index),
+                           result);
+
+            return result;
+        }
+        else
+        {
+            if (difference < 0)
+                stopLow = stopMiddle + 1;
+            else
+                stopHigh = stopMiddle;
+        }
+    }
+
+    /* Not found */
+    if (!res)
+        pfree(result);
+
+    return NULL;
+}
+
 /*
  * Find value in object (i.e. the "value" part of some key/value pair in an
  * object), or find a matching element if we're looking through an array.  Do
@@ -325,88 +423,30 @@ compareJsonbContainers(JsonbContainer *a, JsonbContainer *b)
  */
 JsonbValue *
 findJsonbValueFromContainer(JsonbContainer *container, uint32 flags,
-                            JsonbValue *key)
+                            JsonbValue *key, JsonbValue *res)
 {
-    JEntry       *children = container->children;
     int            count = JsonContainerSize(container);
-    JsonbValue *result;
 
     Assert((flags & ~(JB_FARRAY | JB_FOBJECT)) == 0);
 
-    /* Quick out without a palloc cycle if object/array is empty */
+    /* Quick out if object/array is empty */
     if (count <= 0)
         return NULL;
 
-    result = palloc(sizeof(JsonbValue));
-
     if ((flags & JB_FARRAY) && JsonContainerIsArray(container))
     {
-        char       *base_addr = (char *) (children + count);
-        uint32        offset = 0;
-        int            i;
-
-        for (i = 0; i < count; i++)
-        {
-            fillJsonbValue(container, i, base_addr, offset, result);
-
-            if (key->type == result->type)
-            {
-                if (equalsJsonbScalarValue(key, result))
-                    return result;
-            }
-
-            JBE_ADVANCE_OFFSET(offset, children[i]);
-        }
+        return findJsonbElementInArray(container, key, res);
     }
     else if ((flags & JB_FOBJECT) && JsonContainerIsObject(container))
     {
-        /* Since this is an object, account for *Pairs* of Jentrys */
-        char       *base_addr = (char *) (children + count * 2);
-        uint32        stopLow = 0,
-                    stopHigh = count;
-
         /* Object key passed by caller must be a string */
         Assert(key->type == jbvString);
 
-        /* Binary search on object/pair keys *only* */
-        while (stopLow < stopHigh)
-        {
-            uint32        stopMiddle;
-            int            difference;
-            JsonbValue    candidate;
-
-            stopMiddle = stopLow + (stopHigh - stopLow) / 2;
-
-            candidate.type = jbvString;
-            candidate.val.string.val =
-                base_addr + getJsonbOffset(container, stopMiddle);
-            candidate.val.string.len = getJsonbLength(container, stopMiddle);
-
-            difference = lengthCompareJsonbStringValue(&candidate, key);
-
-            if (difference == 0)
-            {
-                /* Found our key, return corresponding value */
-                int            index = stopMiddle + count;
-
-                fillJsonbValue(container, index, base_addr,
-                               getJsonbOffset(container, index),
-                               result);
-
-                return result;
-            }
-            else
-            {
-                if (difference < 0)
-                    stopLow = stopMiddle + 1;
-                else
-                    stopHigh = stopMiddle;
-            }
-        }
+        return findJsonbKeyInObject(container, key->val.string.val,
+                                    key->val.string.len, res);
     }
 
     /* Not found */
-    pfree(result);
     return NULL;
 }
 
@@ -416,9 +456,9 @@ findJsonbValueFromContainer(JsonbContainer *container, uint32 flags,
  * Returns palloc()'d copy of the value, or NULL if it does not exist.
  */
 JsonbValue *
-getIthJsonbValueFromContainer(JsonbContainer *container, uint32 i)
+getIthJsonbValueFromContainer(JsonbContainer *container, uint32 i,
+                              JsonbValue *result)
 {
-    JsonbValue *result;
     char       *base_addr;
     uint32        nelements;
 
@@ -431,7 +471,8 @@ getIthJsonbValueFromContainer(JsonbContainer *container, uint32 i)
     if (i >= nelements)
         return NULL;
 
-    result = palloc(sizeof(JsonbValue));
+    if (!result)
+        result = palloc(sizeof(JsonbValue));
 
     fillJsonbValue(container, i, base_addr,
                    getJsonbOffset(container, i),
@@ -1009,6 +1050,7 @@ JsonbDeepContains(JsonbIterator **val, JsonbIterator **mContained)
         for (;;)
         {
             JsonbValue *lhsVal; /* lhsVal is from pair in lhs object */
+            JsonbValue    lhsValBuf;
 
             rcont = JsonbIteratorNext(mContained, &vcontained, false);
 
@@ -1021,11 +1063,13 @@ JsonbDeepContains(JsonbIterator **val, JsonbIterator **mContained)
                 return true;
 
             Assert(rcont == WJB_KEY);
+            Assert(vcontained.type == jbvString);
 
             /* First, find value by key... */
-            lhsVal = findJsonbValueFromContainer((*val)->container,
-                                                 JB_FOBJECT,
-                                                 &vcontained);
+            lhsVal = findJsonbKeyInObject((*val)->container,
+                                          vcontained.val.string.val,
+                                          vcontained.val.string.len,
+                                          &lhsValBuf);
 
             if (!lhsVal)
                 return false;
@@ -1126,9 +1170,10 @@ JsonbDeepContains(JsonbIterator **val, JsonbIterator **mContained)
 
             if (IsAJsonbScalar(&vcontained))
             {
-                if (!findJsonbValueFromContainer((*val)->container,
-                                                 JB_FARRAY,
-                                                 &vcontained))
+                JsonbValue    elemBuf;
+
+                if (!findJsonbElementInArray((*val)->container, &vcontained,
+                                             &elemBuf))
                     return false;
             }
             else
@@ -1754,6 +1799,15 @@ convertJsonbScalar(StringInfo buffer, JEntry *jentry, JsonbValue *scalarVal)
     }
 }
 
+static int
+lengthCompareJsonbString(const char *val1, int len1, const char *val2, int len2)
+{
+    if (len1 == len2)
+        return memcmp(val1, val2, len1);
+    else
+        return len1 > len2 ? 1 : -1;
+}
+
 /*
  * Compare two jbvString JsonbValue values, a and b.
  *
@@ -1771,21 +1825,12 @@ lengthCompareJsonbStringValue(const void *a, const void *b)
 {
     const JsonbValue *va = (const JsonbValue *) a;
     const JsonbValue *vb = (const JsonbValue *) b;
-    int            res;
 
     Assert(va->type == jbvString);
     Assert(vb->type == jbvString);
 
-    if (va->val.string.len == vb->val.string.len)
-    {
-        res = memcmp(va->val.string.val, vb->val.string.val, va->val.string.len);
-    }
-    else
-    {
-        res = (va->val.string.len > vb->val.string.len) ? 1 : -1;
-    }
-
-    return res;
+    return lengthCompareJsonbString(va->val.string.val, va->val.string.len,
+                                    vb->val.string.val, vb->val.string.len);
 }
 
 /*
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index fe351edb2b..3497f3ba12 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -454,12 +454,6 @@ static Datum populate_array(ArrayIOData *aio, const char *colname,
 static Datum populate_domain(DomainIOData *io, Oid typid, const char *colname,
                              MemoryContext mcxt, JsValue *jsv, bool isnull);
 
-/* Worker that takes care of common setup for us */
-static JsonbValue *findJsonbValueFromContainerLen(JsonbContainer *container,
-                                                  uint32 flags,
-                                                  char *key,
-                                                  uint32 keylen);
-
 /* functions supporting jsonb_delete, jsonb_set and jsonb_concat */
 static JsonbValue *IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
                                   JsonbParseState **state);
@@ -718,13 +712,15 @@ jsonb_object_field(PG_FUNCTION_ARGS)
     Jsonb       *jb = PG_GETARG_JSONB_P(0);
     text       *key = PG_GETARG_TEXT_PP(1);
     JsonbValue *v;
+    JsonbValue    vbuf;
 
     if (!JB_ROOT_IS_OBJECT(jb))
         PG_RETURN_NULL();
 
-    v = findJsonbValueFromContainerLen(&jb->root, JB_FOBJECT,
-                                       VARDATA_ANY(key),
-                                       VARSIZE_ANY_EXHDR(key));
+    v = findJsonbKeyInObject(&jb->root,
+                             VARDATA_ANY(key),
+                             VARSIZE_ANY_EXHDR(key),
+                             &vbuf);
 
     if (v != NULL)
         PG_RETURN_JSONB_P(JsonbValueToJsonb(v));
@@ -748,53 +744,65 @@ json_object_field_text(PG_FUNCTION_ARGS)
         PG_RETURN_NULL();
 }
 
+static text *
+JsonbValueAsText(JsonbValue *v)
+{
+    switch (v->type)
+    {
+        case jbvNull:
+            return NULL;
+
+        case jbvBool:
+            return v->val.boolean ?
+                cstring_to_text_with_len("true", 4) :
+                cstring_to_text_with_len("false", 5);
+
+        case jbvString:
+            return cstring_to_text_with_len(v->val.string.val,
+                                            v->val.string.len);
+
+        case jbvNumeric:
+            {
+                Datum        cstr = DirectFunctionCall1(numeric_out,
+                                                       PointerGetDatum(v->val.numeric));
+
+                return cstring_to_text(DatumGetCString(cstr));
+            }
+
+        case jbvBinary:
+            {
+                StringInfoData jtext;
+
+                initStringInfo(&jtext);
+                (void) JsonbToCString(&jtext, v->val.binary.data, -1);
+
+                return cstring_to_text_with_len(jtext.data, jtext.len);
+            }
+
+        default:
+            elog(ERROR, "unrecognized jsonb type: %d", (int) v->type);
+            return NULL;
+    }
+}
+
 Datum
 jsonb_object_field_text(PG_FUNCTION_ARGS)
 {
     Jsonb       *jb = PG_GETARG_JSONB_P(0);
     text       *key = PG_GETARG_TEXT_PP(1);
     JsonbValue *v;
+    JsonbValue    vbuf;
 
     if (!JB_ROOT_IS_OBJECT(jb))
         PG_RETURN_NULL();
 
-    v = findJsonbValueFromContainerLen(&jb->root, JB_FOBJECT,
-                                       VARDATA_ANY(key),
-                                       VARSIZE_ANY_EXHDR(key));
+    v = findJsonbKeyInObject(&jb->root,
+                             VARDATA_ANY(key),
+                             VARSIZE_ANY_EXHDR(key),
+                             &vbuf);
 
-    if (v != NULL)
-    {
-        text       *result = NULL;
-
-        switch (v->type)
-        {
-            case jbvNull:
-                break;
-            case jbvBool:
-                result = cstring_to_text(v->val.boolean ? "true" : "false");
-                break;
-            case jbvString:
-                result = cstring_to_text_with_len(v->val.string.val, v->val.string.len);
-                break;
-            case jbvNumeric:
-                result = cstring_to_text(DatumGetCString(DirectFunctionCall1(numeric_out,
-                                                                             PointerGetDatum(v->val.numeric))));
-                break;
-            case jbvBinary:
-                {
-                    StringInfo    jtext = makeStringInfo();
-
-                    (void) JsonbToCString(jtext, v->val.binary.data, -1);
-                    result = cstring_to_text_with_len(jtext->data, jtext->len);
-                }
-                break;
-            default:
-                elog(ERROR, "unrecognized jsonb type: %d", (int) v->type);
-        }
-
-        if (result)
-            PG_RETURN_TEXT_P(result);
-    }
+    if (v != NULL && v->type != jbvNull)
+        PG_RETURN_TEXT_P(JsonbValueAsText(v));
 
     PG_RETURN_NULL();
 }
@@ -820,6 +828,7 @@ jsonb_array_element(PG_FUNCTION_ARGS)
     Jsonb       *jb = PG_GETARG_JSONB_P(0);
     int            element = PG_GETARG_INT32(1);
     JsonbValue *v;
+    JsonbValue    vbuf;
 
     if (!JB_ROOT_IS_ARRAY(jb))
         PG_RETURN_NULL();
@@ -835,7 +844,7 @@ jsonb_array_element(PG_FUNCTION_ARGS)
             element += nelements;
     }
 
-    v = getIthJsonbValueFromContainer(&jb->root, element);
+    v = getIthJsonbValueFromContainer(&jb->root, element, &vbuf);
     if (v != NULL)
         PG_RETURN_JSONB_P(JsonbValueToJsonb(v));
 
@@ -863,6 +872,7 @@ jsonb_array_element_text(PG_FUNCTION_ARGS)
     Jsonb       *jb = PG_GETARG_JSONB_P(0);
     int            element = PG_GETARG_INT32(1);
     JsonbValue *v;
+    JsonbValue    vbuf;
 
     if (!JB_ROOT_IS_ARRAY(jb))
         PG_RETURN_NULL();
@@ -878,40 +888,10 @@ jsonb_array_element_text(PG_FUNCTION_ARGS)
             element += nelements;
     }
 
-    v = getIthJsonbValueFromContainer(&jb->root, element);
-    if (v != NULL)
-    {
-        text       *result = NULL;
-
-        switch (v->type)
-        {
-            case jbvNull:
-                break;
-            case jbvBool:
-                result = cstring_to_text(v->val.boolean ? "true" : "false");
-                break;
-            case jbvString:
-                result = cstring_to_text_with_len(v->val.string.val, v->val.string.len);
-                break;
-            case jbvNumeric:
-                result = cstring_to_text(DatumGetCString(DirectFunctionCall1(numeric_out,
-                                                                             PointerGetDatum(v->val.numeric))));
-                break;
-            case jbvBinary:
-                {
-                    StringInfo    jtext = makeStringInfo();
-
-                    (void) JsonbToCString(jtext, v->val.binary.data, -1);
-                    result = cstring_to_text_with_len(jtext->data, jtext->len);
-                }
-                break;
-            default:
-                elog(ERROR, "unrecognized jsonb type: %d", (int) v->type);
-        }
+    v = getIthJsonbValueFromContainer(&jb->root, element, &vbuf);
 
-        if (result)
-            PG_RETURN_TEXT_P(result);
-    }
+    if (v != NULL && v->type != jbvNull)
+        PG_RETURN_TEXT_P(JsonbValueAsText(v));
 
     PG_RETURN_NULL();
 }
@@ -1389,7 +1369,6 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
 {
     Jsonb       *jb = PG_GETARG_JSONB_P(0);
     ArrayType  *path = PG_GETARG_ARRAYTYPE_P(1);
-    Jsonb       *res;
     Datum       *pathtext;
     bool       *pathnulls;
     int            npath;
@@ -1397,7 +1376,7 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
     bool        have_object = false,
                 have_array = false;
     JsonbValue *jbvp = NULL;
-    JsonbValue    tv;
+    JsonbValue    jbvbuf;
     JsonbContainer *container;
 
     /*
@@ -1425,7 +1404,7 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
         Assert(JB_ROOT_IS_ARRAY(jb) && JB_ROOT_IS_SCALAR(jb));
         /* Extract the scalar value, if it is what we'll return */
         if (npath <= 0)
-            jbvp = getIthJsonbValueFromContainer(container, 0);
+            jbvp = getIthJsonbValueFromContainer(container, 0, &jbvbuf);
     }
 
     /*
@@ -1455,10 +1434,10 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
     {
         if (have_object)
         {
-            jbvp = findJsonbValueFromContainerLen(container,
-                                                  JB_FOBJECT,
-                                                  VARDATA(pathtext[i]),
-                                                  VARSIZE(pathtext[i]) - VARHDRSZ);
+            jbvp = findJsonbKeyInObject(container,
+                                        VARDATA(pathtext[i]),
+                                        VARSIZE(pathtext[i]) - VARHDRSZ,
+                                        &jbvbuf);
         }
         else if (have_array)
         {
@@ -1494,7 +1473,7 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
                     index = nelements + lindex;
             }
 
-            jbvp = getIthJsonbValueFromContainer(container, index);
+            jbvp = getIthJsonbValueFromContainer(container, index, &jbvbuf);
         }
         else
         {
@@ -1509,41 +1488,30 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
 
         if (jbvp->type == jbvBinary)
         {
-            JsonbIterator *it = JsonbIteratorInit((JsonbContainer *) jbvp->val.binary.data);
-            JsonbIteratorToken r;
-
-            r = JsonbIteratorNext(&it, &tv, true);
-            container = (JsonbContainer *) jbvp->val.binary.data;
-            have_object = r == WJB_BEGIN_OBJECT;
-            have_array = r == WJB_BEGIN_ARRAY;
+            container = jbvp->val.binary.data;
+            have_object = JsonContainerIsObject(container);
+            have_array = JsonContainerIsArray(container);
+            Assert(!JsonContainerIsScalar(container));
         }
         else
         {
-            have_object = jbvp->type == jbvObject;
-            have_array = jbvp->type == jbvArray;
+            Assert(IsAJsonbScalar(jbvp));
+            have_object = false;
+            have_array = false;
         }
     }
 
     if (as_text)
     {
-        /* special-case outputs for string and null values */
-        if (jbvp->type == jbvString)
-            PG_RETURN_TEXT_P(cstring_to_text_with_len(jbvp->val.string.val,
-                                                      jbvp->val.string.len));
         if (jbvp->type == jbvNull)
             PG_RETURN_NULL();
-    }
-
-    res = JsonbValueToJsonb(jbvp);
 
-    if (as_text)
-    {
-        PG_RETURN_TEXT_P(cstring_to_text(JsonbToCString(NULL,
-                                                        &res->root,
-                                                        VARSIZE(res))));
+        PG_RETURN_TEXT_P(JsonbValueAsText(jbvp));
     }
     else
     {
+        Jsonb       *res = JsonbValueToJsonb(jbvp);
+
         /* not text mode - just hand back the jsonb */
         PG_RETURN_JSONB_P(res);
     }
@@ -1760,24 +1728,7 @@ each_worker_jsonb(FunctionCallInfo fcinfo, const char *funcname, bool as_text)
                 }
                 else
                 {
-                    text       *sv;
-
-                    if (v.type == jbvString)
-                    {
-                        /* In text mode, scalar strings should be dequoted */
-                        sv = cstring_to_text_with_len(v.val.string.val, v.val.string.len);
-                    }
-                    else
-                    {
-                        /* Turn anything else into a json string */
-                        StringInfo    jtext = makeStringInfo();
-                        Jsonb       *jb = JsonbValueToJsonb(&v);
-
-                        (void) JsonbToCString(jtext, &jb->root, 0);
-                        sv = cstring_to_text_with_len(jtext->data, jtext->len);
-                    }
-
-                    values[1] = PointerGetDatum(sv);
+                    values[1] = PointerGetDatum(JsonbValueAsText(&v));
                 }
             }
             else
@@ -2070,24 +2021,7 @@ elements_worker_jsonb(FunctionCallInfo fcinfo, const char *funcname,
                 }
                 else
                 {
-                    text       *sv;
-
-                    if (v.type == jbvString)
-                    {
-                        /* in text mode scalar strings should be dequoted */
-                        sv = cstring_to_text_with_len(v.val.string.val, v.val.string.len);
-                    }
-                    else
-                    {
-                        /* turn anything else into a json string */
-                        StringInfo    jtext = makeStringInfo();
-                        Jsonb       *jb = JsonbValueToJsonb(&v);
-
-                        (void) JsonbToCString(jtext, &jb->root, 0);
-                        sv = cstring_to_text_with_len(jtext->data, jtext->len);
-                    }
-
-                    values[0] = PointerGetDatum(sv);
+                    values[0] = PointerGetDatum(JsonbValueAsText(&v));
                 }
             }
 
@@ -3086,8 +3020,8 @@ JsObjectGetField(JsObject *obj, char *field, JsValue *jsv)
     else
     {
         jsv->val.jsonb = !obj->val.jsonb_cont ? NULL :
-            findJsonbValueFromContainerLen(obj->val.jsonb_cont, JB_FOBJECT,
-                                           field, strlen(field));
+            findJsonbKeyInObject(obj->val.jsonb_cont, field, strlen(field),
+                                 NULL);
 
         return jsv->val.jsonb != NULL;
     }
@@ -3899,22 +3833,6 @@ populate_recordset_object_field_end(void *state, char *fname, bool isnull)
     }
 }
 
-/*
- * findJsonbValueFromContainer() wrapper that sets up JsonbValue key string.
- */
-static JsonbValue *
-findJsonbValueFromContainerLen(JsonbContainer *container, uint32 flags,
-                               char *key, uint32 keylen)
-{
-    JsonbValue    k;
-
-    k.type = jbvString;
-    k.val.string.val = key;
-    k.val.string.len = keylen;
-
-    return findJsonbValueFromContainer(container, flags, &k);
-}
-
 /*
  * Semantic actions for json_strip_nulls.
  *
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index d8647f71af..60a3888bf8 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -585,7 +585,7 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt, JsonPathItem *jsp,
                 key.val.string.val = jspGetString(jsp, &key.val.string.len);
 
                 v = findJsonbValueFromContainer(jb->val.binary.data,
-                                                JB_FOBJECT, &key);
+                                                JB_FOBJECT, &key, NULL);
 
                 if (v != NULL)
                 {
@@ -717,7 +717,7 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt, JsonPathItem *jsp,
                         else
                         {
                             v = getIthJsonbValueFromContainer(jb->val.binary.data,
-                                                              (uint32) index);
+                                                              (uint32) index, NULL);
 
                             if (v == NULL)
                                 continue;
@@ -1935,7 +1935,7 @@ getJsonPathVariable(JsonPathExecContext *cxt, JsonPathItem *variable,
     tmp.val.string.val = varName;
     tmp.val.string.len = varNameLength;
 
-    v = findJsonbValueFromContainer(&vars->root, JB_FOBJECT, &tmp);
+    v = findJsonbValueFromContainer(&vars->root, JB_FOBJECT, &tmp, NULL);
 
     if (v)
     {
diff --git a/src/include/utils/jsonb.h b/src/include/utils/jsonb.h
index ac52b75f51..ee76f34d83 100644
--- a/src/include/utils/jsonb.h
+++ b/src/include/utils/jsonb.h
@@ -361,11 +361,12 @@ typedef struct JsonbIterator
 extern uint32 getJsonbOffset(const JsonbContainer *jc, int index);
 extern uint32 getJsonbLength(const JsonbContainer *jc, int index);
 extern int    compareJsonbContainers(JsonbContainer *a, JsonbContainer *b);
+extern JsonbValue *findJsonbKeyInObject(JsonbContainer *container, const char *keyVal,
+                                        int keyLen, JsonbValue *res);
 extern JsonbValue *findJsonbValueFromContainer(JsonbContainer *sheader,
-                                               uint32 flags,
-                                               JsonbValue *key);
+                                               uint32 flags, JsonbValue *key, JsonbValue *res);
 extern JsonbValue *getIthJsonbValueFromContainer(JsonbContainer *sheader,
-                                                 uint32 i);
+                                                 uint32 i, JsonbValue *result);
 extern JsonbValue *pushJsonbValue(JsonbParseState **pstate,
                                   JsonbIteratorToken seq, JsonbValue *jbval);
 extern JsonbIterator *JsonbIteratorInit(JsonbContainer *container);

Re: Optimization of some jsonb functions

От
Alvaro Herrera
Дата:
On 2019-Jul-26, Joe Nelson wrote:

> Thomas Munro wrote:
> > This doesn't apply -- to attract reviewers, could we please have a rebase?
> 
> To help the review go forward, I have rebased the patch on 27cd521e6e.
> It passes `make check` for me, but that's as far as I've verified the
> correctness.
> 
> I squashed the changes into a single patch, sorry if that makes it
> harder to review than the original set of five patch files...

Well, I think that was useless, so I rebased again -- attached.
(Thanks, git-imerge).

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Вложения

Re: Optimization of some jsonb functions

От
Alvaro Herrera
Дата:
On 2019-Sep-18, Alvaro Herrera wrote:

> Well, I think that was useless, so I rebased again -- attached.

... which is how you find out that 0001 as an independent patch is not
really a valid one, since it depends on an API change that does not
happen until 0005.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: Optimization of some jsonb functions

От
Alvaro Herrera
Дата:
On 2019-Sep-19, Alvaro Herrera wrote:

> On 2019-Sep-18, Alvaro Herrera wrote:
> 
> > Well, I think that was useless, so I rebased again -- attached.
> 
> ... which is how you find out that 0001 as an independent patch is not
> really a valid one, since it depends on an API change that does not
> happen until 0005.

... and there were other compilation problems too, presumably fixed
silently by Joe in his rebase, but which I fixed again for this series
which now seems more credible.  I tested compile and regression tests
after each patch, it all works locally.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Вложения

Re: Optimization of some jsonb functions

От
Alvaro Herrera
Дата:
I pushed the first few parts.  The attached is a rebased copy of the
last remaining piece.  However, I didn't quite understand what this was
doing, so I refrained from pushing.  I think there are two patches here:
one that adapts the API of findJsonbValueFromContainer and
getIthJsonbValueFromContainer to take the output result pointer as an
argument, allowing to save palloc cycles just like the newly added
getKeyJsonValueFromContainer(); and the other changes JsonbDeepContains
so that it uses a new function (which is a function with a weird API
that would be extracted from findJsonbValueFromContainer).

Also, the current patch just passes NULL into the routines from
jsonpath_exec.c but I think it would be useful to pass pointers into
stack-allocated result structs instead, at least in getJsonPathVariable.

Since the majority of this patchset got pushed, I'll leave this for
Nikita to handle for the next commitfest if he wants to, and mark this
CF entry as committed.

Thanks!

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Вложения