From: Peter Geoghegan Date: Tue, 18 Mar 2014 06:26:42 +0000 (-0700) Subject: Consolidate and simplify searching and comparison logic X-Git-Url: http://git.postgresql.org/gitweb/backend_dirs.html?a=commitdiff_plain;h=0ce84d52bdb521f64217c49986deed5a986303b2;p=users%2Fandresfreund%2Fpostgres.git Consolidate and simplify searching and comparison logic Almost ever comparison can go through compareJsonbScalarValue() directly, simplifying things. --- diff --git a/src/backend/utils/adt/jsonb_util.c b/src/backend/utils/adt/jsonb_util.c index 8db140612a..34adef4b41 100644 --- a/src/backend/utils/adt/jsonb_util.c +++ b/src/backend/utils/adt/jsonb_util.c @@ -49,7 +49,7 @@ typedef struct convertState } convertState; -static bool compareJsonbScalarValue(JsonbValue * a, JsonbValue * b); +static int compareJsonbScalarValue(JsonbValue * a, JsonbValue * b); static int lexicalCompareJsonbStringValue(const void *a, const void *b); static Size convertJsonb(JsonbValue * v, Jsonb* buffer); static void walkJsonbValueConversion(JsonbValue * value, convertState * state, @@ -170,20 +170,13 @@ compareJsonbSuperHeaderValue(JsonbSuperHeader a, JsonbSuperHeader b) { switch (v1.type) { - case jbvNull: - res = 0; - break; case jbvString: res = lexicalCompareJsonbStringValue(&v1, &v2); break; + case jbvNull: case jbvNumeric: - res = DatumGetInt32(DirectFunctionCall2(numeric_cmp, - PointerGetDatum(v1.numeric), - PointerGetDatum(v2.numeric))); - break; case jbvBool: - if (v1.boolean != v2.boolean) - res = (v1.boolean > v2.boolean) ? 1 : -1; + res = compareJsonbScalarValue(&v1, &v2); break; case jbvArray: /* @@ -307,57 +300,33 @@ findJsonbValueFromSuperHeader(JsonbSuperHeader sheader, uint32 flags, { r->type = jbvNull; r->estSize = sizeof(JEntry); - if (lowbound) - *lowbound = i; - - return r; } else if (JBE_ISSTRING(*e) && key->type == jbvString) { - /* Equivalent to lengthCompareJsonbStringValue() */ - if (key->string.len == JBE_LEN(*e) && - memcmp(key->string.val, data + JBE_OFF(*e), - key->string.len) == 0) - { - r->type = jbvString; - r->string.val = data + JBE_OFF(*e); - r->string.len = key->string.len; - r->estSize = sizeof(JEntry) + r->string.len; - if (lowbound) - *lowbound = i; - - return r; - } + r->type = jbvString; + r->string.val = data + JBE_OFF(*e); + r->string.len = key->string.len; + r->estSize = sizeof(JEntry) + r->string.len; } else if (JBE_ISNUMERIC(*e) && key->type == jbvNumeric) { - Numeric entry = (Numeric) (data + INTALIGN(JBE_OFF(*e))); - - if (DatumGetBool(DirectFunctionCall2(numeric_eq, - PointerGetDatum(entry), - PointerGetDatum(key->numeric)))) - { - r->type = jbvNumeric; - r->numeric = entry; - if (lowbound) - *lowbound = i; - - return r; - } + r->type = jbvNumeric; + r->numeric = (Numeric) (data + INTALIGN(JBE_OFF(*e))); } else if (JBE_ISBOOL(*e) && key->type == jbvBool) { - if ((JBE_ISBOOL_TRUE(*e) && key->boolean) || - (JBE_ISBOOL_FALSE(*e) && !key->boolean)) - { - r->type = jbvBool; - r->boolean = JBE_ISBOOL_TRUE(*e) != 0; - r->estSize = sizeof(JEntry); - if (lowbound) - *lowbound = i; + r->type = jbvBool; + r->boolean = JBE_ISBOOL_TRUE(*e) != 0; + r->estSize = sizeof(JEntry); + } + else + continue; - return r; - } + if (compareJsonbScalarValue(key, r) == 0) + { + if (lowbound) + *lowbound = i; + return r; } } } @@ -371,16 +340,12 @@ findJsonbValueFromSuperHeader(JsonbSuperHeader sheader, uint32 flags, /* Object key past by caller must be a string */ Assert(key->type == jbvString); - /* - * Binary search for matching jsonb value using "inner comparator" - * logic (i.e. not doing lexical comparisons). - * - * Searching through object's pair "keys" *only*. - */ + /* Binary search on object/pair keys *only* */ while (stopLow < count) { JEntry *entry; int difference; + JsonbValue candidate; /* * Note how we compensate for the fact that we're iterating through @@ -390,12 +355,12 @@ findJsonbValueFromSuperHeader(JsonbSuperHeader sheader, uint32 flags, entry = array + stopMiddle * 2; - /* Equivalent to lengthCompareJsonbStringValue() */ - if (key->string.len == JBE_LEN(*entry)) - difference = memcmp(data + JBE_OFF(*entry), key->string.val, - key->string.len); - else - difference = (JBE_LEN(*entry) > key->string.len) ? 1 : -1; + candidate.type = jbvString; + candidate.string.val = data + JBE_OFF(*entry); + candidate.string.len = JBE_LEN(*entry); + candidate.estSize = sizeof(JEntry) + candidate.string.len; + + difference = lengthCompareJsonbStringValue(&candidate, key, NULL); if (difference == 0) { @@ -823,7 +788,7 @@ deepContains(JsonbIterator ** val, JsonbIterator ** mContained) /* * When we get through caller's rhs "is it contained within?" - * object without failing to find some value, we're done. + * object without failing to find one of its values, we're done. */ if (rcont == WJB_END_OBJECT) return true; @@ -839,18 +804,25 @@ deepContains(JsonbIterator ** val, JsonbIterator ** mContained) if (!lhsVal) return false; - /* ...at this stage we know that there is at least a key match */ + /* + * ...at this stage we know that there is at least a key match for + * this rhs pair. + */ rcont = JsonbIteratorNext(mContained, &vcontained, true); Assert(rcont == WJB_VALUE); + /* + * Compare rhs pair's value with lhs pair's value just found using + * key + */ if (lhsVal->type != vcontained.type) { return false; } else if (lhsVal->type >= jbvNull && lhsVal->type < jbvArray) { - if (!compareJsonbScalarValue(lhsVal, &vcontained)) + if (compareJsonbScalarValue(lhsVal, &vcontained) != 0) return false; } else @@ -864,6 +836,13 @@ deepContains(JsonbIterator ** val, JsonbIterator ** mContained) nestval = JsonbIteratorInit(lhsVal->binary.data); nestContained = JsonbIteratorInit(vcontained.binary.data); + /* + * Match "value" side of rhs datum object's pair recursively. + * It's a nested structure. + * + * Note that nesting still has to "match up" at the right + * nesting sub-levels. + */ if (!deepContains(&nestval, &nestContained)) return false; } @@ -905,7 +884,11 @@ deepContains(JsonbIterator ** val, JsonbIterator ** mContained) { uint32 i; - if (rhsVals == NULL) /* ...this is first container found in rhs array? */ + /* + * If this is first container found in rhs array (at this + * depth), initialize temp array + */ + if (rhsVals == NULL) { uint32 j = 0; @@ -922,6 +905,7 @@ deepContains(JsonbIterator ** val, JsonbIterator ** mContained) rhsVals[j++] = vval; } + /* No container elements in temp array, so give up now */ if (j == 0) return false; @@ -1050,16 +1034,13 @@ hash_scalar_value(const JsonbValue * v, uint32 * hash_state) } /* - * Are two scalar JsonbValues a and b equal? + * Are two scalar JsonbValues of the same type a and b equal? * * Does not use lexical comparisons. Therefore, it is essentially that this - * never be used for anything other than searching for values within a single - * jsonb. - * - * We return bool because we don't want to give anyone any ideas about using - * this for sorting. This is just for "contains" style searching. + * never be used against Strings for anything other than searching for values + * within a single jsonb. */ -static bool +static int compareJsonbScalarValue(JsonbValue * a, JsonbValue * b) { if (a->type == b->type) @@ -1067,21 +1048,23 @@ compareJsonbScalarValue(JsonbValue * a, JsonbValue * b) switch (a->type) { case jbvNull: - return true; + return 0; case jbvString: - return lengthCompareJsonbStringValue(a, b, NULL) == 0; + return lengthCompareJsonbStringValue(a, b, NULL); case jbvBool: - return a->boolean == b->boolean; + if (a->boolean != b->boolean) + return (a->boolean > b->boolean) ? 1 : -1; + else + return 0; case jbvNumeric: - return DatumGetBool(DirectFunctionCall2(numeric_eq, - PointerGetDatum(a->numeric), - PointerGetDatum(b->numeric))); + return DatumGetInt32(DirectFunctionCall2(numeric_cmp, + PointerGetDatum(a->numeric), + PointerGetDatum(b->numeric))); default: elog(ERROR, "invalid jsonb scalar type"); } } - - return false; + elog(ERROR, "jsonb scalar type mismatch"); } /*