Re: Optimization of some jsonb functions - Mailing list pgsql-hackers

From Joe Nelson
Subject Re: Optimization of some jsonb functions
Date
Msg-id 20190726193422.GA82289@begriffs.com
Whole thread Raw
In response to Re: Optimization of some jsonb functions  (Thomas Munro <thomas.munro@gmail.com>)
Responses Re: Optimization of some jsonb functions  (Alvaro Herrera <alvherre@2ndquadrant.com>)
List pgsql-hackers
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);

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: seems like a bug in pgbench -R
Next
From: Tom Lane
Date:
Subject: Re: proposal: type info support functions for functions that use "any" type