On 05/18/2015 08:17 AM, Andrew Dunstan wrote:
On 05/17/2015 09:08 PM, Peter Geoghegan wrote:
On Sun, May 17, 2015 at 8:04 AM, Andrew Dunstan <and...@dunslane.net>
wrote:
Sure. I thought we'd covered this but it's possible that we didn't,
or that
it got rebroken. There have been complaints about the limitation on
values
containing jbvBinary, so let's just remove it if that can be done
simply, as
it seems to be a not uncommonly encountered problem.
Are you going to submit a patch for that?
I'll try and come up with something. It's not a trivial fix.
Here's a thought. in pushJsonbValue() the value pushed is called
scalarVal, and in all our regression tests it is in fact scalar.
However, the Asserts inside the function tell a different story:
case WJB_VALUE:
Assert(IsAJsonbScalar(scalarVal) ||
scalarVal->type == jbvBinary);
appendValue(*pstate, scalarVal);
break;
case WJB_ELEM:
Assert(IsAJsonbScalar(scalarVal) ||
scalarVal->type == jbvBinary);
appendElement(*pstate, scalarVal);
break;
and indeed it turns out that your delete example does push a value
with jbvBinary, thus triggering the problem. So, could we deal with
that, by, say, setting up an iterator for the binary datum and just
pushing all of its values?
Here's an patch along those lines. It seems to do the trick, at least
for your test case, and it has the merit of being very small, so small
I'd like to backpatch it - accepting jbvBinary as is in pushJsonbValue
seems like a bug to me.
cheers
andrew
diff --git a/src/backend/utils/adt/jsonb_util.c b/src/backend/utils/adt/jsonb_util.c
index 5cd9140..03cc125 100644
--- a/src/backend/utils/adt/jsonb_util.c
+++ b/src/backend/utils/adt/jsonb_util.c
@@ -57,6 +57,9 @@ 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 void uniqueifyJsonbObject(JsonbValue *object);
+static JsonbValue *pushJsonbValueScalar(JsonbParseState **pstate,
+ JsonbIteratorToken seq,
+ JsonbValue *scalarVal);
/*
* Turn an in-memory JsonbValue into a Jsonb for on-disk storage.
@@ -505,8 +508,37 @@ fillJsonbValue(JsonbContainer *container, int index,
* JsonbValue. There is one exception -- WJB_BEGIN_ARRAY callers may pass a
* "raw scalar" pseudo array to append that.
*/
+
JsonbValue *
pushJsonbValue(JsonbParseState **pstate, JsonbIteratorToken seq,
+ JsonbValue *jbval)
+{
+ JsonbIterator *it;
+ JsonbValue *res = NULL;
+ JsonbValue v;
+ JsonbIteratorToken tok;
+
+ if (!jbval || (seq != WJB_ELEM && seq != WJB_VALUE) ||
+ jbval->type != jbvBinary)
+ {
+ /* drop through */
+ return pushJsonbValueScalar(pstate, seq, jbval);
+ }
+
+ /* unpack the binary and add each piece to the pstate */
+ it = JsonbIteratorInit(jbval->val.binary.data);
+ while ((tok = JsonbIteratorNext(&it, &v, false)) != WJB_DONE)
+ res = pushJsonbValueScalar(pstate, tok,
+ tok < WJB_BEGIN_ARRAY ? &v : NULL);
+
+ return res;
+}
+
+
+
+
+static JsonbValue *
+pushJsonbValueScalar(JsonbParseState **pstate, JsonbIteratorToken seq,
JsonbValue *scalarVal)
{
JsonbValue *result = NULL;
@@ -549,13 +581,11 @@ pushJsonbValue(JsonbParseState **pstate, JsonbIteratorToken seq,
appendKey(*pstate, scalarVal);
break;
case WJB_VALUE:
- Assert(IsAJsonbScalar(scalarVal) ||
- scalarVal->type == jbvBinary);
+ Assert(IsAJsonbScalar(scalarVal));
appendValue(*pstate, scalarVal);
break;
case WJB_ELEM:
- Assert(IsAJsonbScalar(scalarVal) ||
- scalarVal->type == jbvBinary);
+ Assert(IsAJsonbScalar(scalarVal));
appendElement(*pstate, scalarVal);
break;
case WJB_END_OBJECT:
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
