pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Joe Nelson <joe@begriffs.com>
Cc: Thomas Munro <thomas.munro@gmail.com>
Cc: David Steele <david@pgmasters.net>
Cc: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
Cc: Nikita Glukhov <n.gluhov@postgrespro.ru>
Cc: PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Subject: Re: Optimization of some jsonb functions
Date: Fri, 20 Sep 2019 21:09:31 -0300
Message-ID: <20190921000931.GA7648@alvherre.pgsql> (raw)
In-Reply-To: <20190919034715.GA6743@alvherre.pgsql>

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

Attachments:

findJsonbElementInArray.patch (text/x-diff, 9.3K) download 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 7e0d9de7f0..7ec33cebf2 100644 --- a/src/backend/utils/adt/jsonb_util.c +++ b/src/backend/utils/adt/jsonb_util.c @@ -33,6 +33,8 @@ #define JSONB_MAX_ELEMS (Min(MaxAllocSize / sizeof(JsonbValue), JB_CMASK)) #define JSONB_MAX_PAIRS (Min(MaxAllocSize / sizeof(JsonbPair), JB_CMASK)) +static JsonbValue *findJsonbElementInArray(JsonbContainer *container, + JsonbValue *elem, JsonbValue *res); static void fillJsonbValue(JsonbContainer *container, int index, char *base_addr, uint32 offset, JsonbValue *result); @@ -327,38 +329,19 @@ 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); 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; if ((flags & JB_FARRAY) && JsonContainerIsArray(container)) { - JsonbValue *result = palloc(sizeof(JsonbValue)); - 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]); - } - - pfree(result); + return findJsonbElementInArray(container, key, res); } else if ((flags & JB_FOBJECT) && JsonContainerIsObject(container)) { @@ -366,13 +349,48 @@ findJsonbValueFromContainer(JsonbContainer *container, uint32 flags, Assert(key->type == jbvString); return getKeyJsonValueFromContainer(container, key->val.string.val, - key->val.string.len, NULL); + key->val.string.len, res); } /* Not found */ return NULL; } +/* + * Subroutine for findJsonbValueFromContainer + * + * 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', which is also * returned. @@ -450,9 +468,9 @@ getKeyJsonValueFromContainer(JsonbContainer *container, * 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; @@ -465,7 +483,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), @@ -1163,9 +1182,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 diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c index 3553a304b8..f75d496102 100644 --- a/src/backend/utils/adt/jsonfuncs.c +++ b/src/backend/utils/adt/jsonfuncs.c @@ -787,6 +787,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(); @@ -802,7 +803,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)); @@ -830,6 +831,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(); @@ -845,7 +847,7 @@ jsonb_array_element_text(PG_FUNCTION_ARGS) element += nelements; } - v = getIthJsonbValueFromContainer(&jb->root, element); + v = getIthJsonbValueFromContainer(&jb->root, element, &vbuf); if (v != NULL && v->type != jbvNull) PG_RETURN_TEXT_P(JsonbValueAsText(v)); @@ -1361,7 +1363,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); } /* @@ -1430,7 +1432,7 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text) index = nelements + lindex; } - jbvp = getIthJsonbValueFromContainer(container, index); + jbvp = getIthJsonbValueFromContainer(container, index, &jbvbuf); } else { diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c index 565b00c426..290b011ef6 100644 --- a/src/backend/utils/adt/jsonpath_exec.c +++ b/src/backend/utils/adt/jsonpath_exec.c @@ -583,7 +583,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) { @@ -715,7 +715,7 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt, JsonPathItem *jsp, else { v = getIthJsonbValueFromContainer(jb->val.binary.data, - (uint32) index); + (uint32) index, NULL); if (v == NULL) continue; @@ -1909,7 +1909,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 35766e106a..2ee6d3f6b4 100644 --- a/src/include/utils/jsonb.h +++ b/src/include/utils/jsonb.h @@ -363,12 +363,13 @@ extern uint32 getJsonbLength(const JsonbContainer *jc, int index); extern int compareJsonbContainers(JsonbContainer *a, JsonbContainer *b); extern JsonbValue *findJsonbValueFromContainer(JsonbContainer *sheader, uint32 flags, - JsonbValue *key); + JsonbValue *key, + JsonbValue *res); extern JsonbValue *getKeyJsonValueFromContainer(JsonbContainer *container, const char *keyVal, int keyLen, JsonbValue *res); extern JsonbValue *getIthJsonbValueFromContainer(JsonbContainer *sheader, - uint32 i); + uint32 i, JsonbValue *res); extern JsonbValue *pushJsonbValue(JsonbParseState **pstate, JsonbIteratorToken seq, JsonbValue *jbval); extern JsonbIterator *JsonbIteratorInit(JsonbContainer *container);

view thread (10+ messages)

Message-ID: <20190921000931.GA7648@alvherre.pgsql>
Permalink:  ../20190921000931.GA7648@alvherre.pgsql/
Also on:    postgresql.org/message-id/20190921000931.GA7648@alvherre.pgsql

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: alvherre@2ndquadrant.com, joe@begriffs.com, thomas.munro@gmail.com, david@pgmasters.net, andrew.dunstan@2ndquadrant.com, n.gluhov@postgrespro.ru
  Subject: Re: Optimization of some jsonb functions
  In-Reply-To: <20190921000931.GA7648@alvherre.pgsql>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox