From: Tom Lane Date: Sun, 9 Jan 2011 21:43:56 +0000 (-0500) Subject: Update contrib/hstore for new GIN extractQuery API. X-Git-Tag: REL9_1_ALPHA4~478 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ba398969cd1b32c550345bcf6b713deeb0a7d1de;p=postgresql Update contrib/hstore for new GIN extractQuery API. In particular, make hstore @> '' succeed for all hstores, likewise hstore ?& '{}'. Previously the results were inconsistent and could depend on whether you were using a GiST index, GIN index, or seqscan. --- diff --git a/contrib/hstore/expected/hstore.out b/contrib/hstore/expected/hstore.out index 19dd299af7..354fff20fe 100644 --- a/contrib/hstore/expected/hstore.out +++ b/contrib/hstore/expected/hstore.out @@ -435,7 +435,7 @@ select hstore 'a=>NULL, b=>qq' ?& ARRAY['c','d']; select hstore 'a=>NULL, b=>qq' ?& '{}'::text[]; ?column? ---------- - f + t (1 row) -- delete diff --git a/contrib/hstore/hstore_gin.c b/contrib/hstore/hstore_gin.c index 8fd3d27873..d55674c79f 100644 --- a/contrib/hstore/hstore_gin.c +++ b/contrib/hstore/hstore_gin.c @@ -10,6 +10,13 @@ #include "hstore.h" +/* + * When using a GIN index for hstore, we choose to index both keys and values. + * The storage format is "text" values, with K, V, or N prepended to the string + * to indicate key, value, or null values. (As of 9.1 it might be better to + * store null values as nulls, but we'll keep it this way for on-disk + * compatibility.) + */ #define KEYFLAG 'K' #define VALFLAG 'V' #define NULLFLAG 'N' @@ -17,14 +24,17 @@ PG_FUNCTION_INFO_V1(gin_extract_hstore); Datum gin_extract_hstore(PG_FUNCTION_ARGS); +/* Build an indexable text value */ static text * -makeitem(char *str, int len) +makeitem(char *str, int len, char flag) { text *item; item = (text *) palloc(VARHDRSZ + len + 1); SET_VARSIZE(item, VARHDRSZ + len + 1); + *VARDATA(item) = flag; + if (str && len > 0) memcpy(VARDATA(item) + 1, str, len); @@ -50,20 +60,15 @@ gin_extract_hstore(PG_FUNCTION_ARGS) { text *item; - item = makeitem(HS_KEY(hsent, ptr, i), HS_KEYLEN(hsent, i)); - *VARDATA(item) = KEYFLAG; + item = makeitem(HS_KEY(hsent, ptr, i), HS_KEYLEN(hsent, i), + KEYFLAG); entries[2 * i] = PointerGetDatum(item); if (HS_VALISNULL(hsent, i)) - { - item = makeitem(NULL, 0); - *VARDATA(item) = NULLFLAG; - } + item = makeitem(NULL, 0, NULLFLAG); else - { - item = makeitem(HS_VAL(hsent, ptr, i), HS_VALLEN(hsent, i)); - *VARDATA(item) = VALFLAG; - } + item = makeitem(HS_VAL(hsent, ptr, i), HS_VALLEN(hsent, i), + VALFLAG); entries[2 * i + 1] = PointerGetDatum(item); } @@ -76,30 +81,31 @@ Datum gin_extract_hstore_query(PG_FUNCTION_ARGS); Datum gin_extract_hstore_query(PG_FUNCTION_ARGS) { + int32 *nentries = (int32 *) PG_GETARG_POINTER(1); StrategyNumber strategy = PG_GETARG_UINT16(2); + int32 *searchMode = (int32 *) PG_GETARG_POINTER(6); + Datum *entries; if (strategy == HStoreContainsStrategyNumber) { - PG_RETURN_DATUM(DirectFunctionCall2(gin_extract_hstore, - PG_GETARG_DATUM(0), - PG_GETARG_DATUM(1) - )); + /* Query is an hstore, so just apply gin_extract_hstore... */ + entries = (Datum *) + DatumGetPointer(DirectFunctionCall2(gin_extract_hstore, + PG_GETARG_DATUM(0), + PointerGetDatum(nentries))); + /* ... except that "contains {}" requires a full index scan */ + if (entries == NULL) + *searchMode = GIN_SEARCH_MODE_ALL; } else if (strategy == HStoreExistsStrategyNumber) { - text *item, - *query = PG_GETARG_TEXT_PP(0); - int32 *nentries = (int32 *) PG_GETARG_POINTER(1); - Datum *entries = NULL; + text *query = PG_GETARG_TEXT_PP(0); + text *item; *nentries = 1; entries = (Datum *) palloc(sizeof(Datum)); - - item = makeitem(VARDATA_ANY(query), VARSIZE_ANY_EXHDR(query)); - *VARDATA(item) = KEYFLAG; + item = makeitem(VARDATA_ANY(query), VARSIZE_ANY_EXHDR(query), KEYFLAG); entries[0] = PointerGetDatum(item); - - PG_RETURN_POINTER(entries); } else if (strategy == HStoreExistsAnyStrategyNumber || strategy == HStoreExistsAllStrategyNumber) @@ -110,8 +116,6 @@ gin_extract_hstore_query(PG_FUNCTION_ARGS) int key_count; int i, j; - int32 *nentries = (int32 *) PG_GETARG_POINTER(1); - Datum *entries = NULL; text *item; deconstruct_array(query, @@ -122,21 +126,25 @@ gin_extract_hstore_query(PG_FUNCTION_ARGS) for (i = 0, j = 0; i < key_count; ++i) { + /* Nulls in the array are ignored, cf hstoreArrayToPairs */ if (key_nulls[i]) continue; - item = makeitem(VARDATA(key_datums[i]), VARSIZE(key_datums[i]) - VARHDRSZ); - *VARDATA(item) = KEYFLAG; + item = makeitem(VARDATA(key_datums[i]), VARSIZE(key_datums[i]) - VARHDRSZ, KEYFLAG); entries[j++] = PointerGetDatum(item); } - *nentries = j ? j : -1; - - PG_RETURN_POINTER(entries); + *nentries = j; + /* ExistsAll with no keys should match everything */ + if (j == 0 && strategy == HStoreExistsAllStrategyNumber) + *searchMode = GIN_SEARCH_MODE_ALL; } else - elog(ERROR, "Unsupported strategy number: %d", strategy); + { + elog(ERROR, "unrecognized strategy number: %d", strategy); + entries = NULL; /* keep compiler quiet */ + } - PG_RETURN_POINTER(NULL); + PG_RETURN_POINTER(entries); } PG_FUNCTION_INFO_V1(gin_consistent_hstore); @@ -154,42 +162,52 @@ gin_consistent_hstore(PG_FUNCTION_ARGS) /* Pointer *extra_data = (Pointer *) PG_GETARG_POINTER(4); */ bool *recheck = (bool *) PG_GETARG_POINTER(5); bool res = true; - - *recheck = false; + int32 i; if (strategy == HStoreContainsStrategyNumber) { - int i; - /* - * Index lost information about correspondence of keys and values, so - * we need recheck (pre-8.4 this is handled at SQL level) + * Index doesn't have information about correspondence of keys and + * values, so we need recheck. However, if not all the keys are + * present, we can fail at once. */ *recheck = true; - for (i = 0; res && i < nkeys; i++) - if (check[i] == false) + for (i = 0; i < nkeys; i++) + { + if (!check[i]) + { res = false; + break; + } + } } else if (strategy == HStoreExistsStrategyNumber) { - /* Existence of key is guaranteed */ + /* Existence of key is guaranteed in default search mode */ + *recheck = false; res = true; } else if (strategy == HStoreExistsAnyStrategyNumber) { - /* Existence of key is guaranteed */ + /* Existence of key is guaranteed in default search mode */ + *recheck = false; res = true; } else if (strategy == HStoreExistsAllStrategyNumber) { - int i; - - for (i = 0; res && i < nkeys; ++i) + /* Testing for all the keys being present gives an exact result */ + *recheck = false; + for (i = 0; i < nkeys; i++) + { if (!check[i]) + { res = false; + break; + } + } } else - elog(ERROR, "Unsupported strategy number: %d", strategy); + elog(ERROR, "unrecognized strategy number: %d", strategy); PG_RETURN_BOOL(res); } diff --git a/contrib/hstore/hstore_op.c b/contrib/hstore/hstore_op.c index 93d8cbcaeb..cb6200ab1d 100644 --- a/contrib/hstore/hstore_op.c +++ b/contrib/hstore/hstore_op.c @@ -168,14 +168,16 @@ hstore_exists_any(PG_FUNCTION_ARGS) * start one entry past the previous "found" entry, or at the lower bound * of the last search. */ - - for (i = 0; !res && i < nkeys; ++i) + for (i = 0; i < nkeys; i++) { int idx = hstoreFindKey(hs, &lowbound, key_pairs[i].key, key_pairs[i].keylen); if (idx >= 0) + { res = true; + break; + } } PG_RETURN_BOOL(res); @@ -193,7 +195,7 @@ hstore_exists_all(PG_FUNCTION_ARGS) Pairs *key_pairs = hstoreArrayToPairs(keys, &nkeys); int i; int lowbound = 0; - bool res = nkeys ? true : false; + bool res = true; /* * we exploit the fact that the pairs list is already sorted into strictly @@ -201,14 +203,16 @@ hstore_exists_all(PG_FUNCTION_ARGS) * start one entry past the previous "found" entry, or at the lower bound * of the last search. */ - - for (i = 0; res && i < nkeys; ++i) + for (i = 0; i < nkeys; i++) { int idx = hstoreFindKey(hs, &lowbound, key_pairs[i].key, key_pairs[i].keylen); if (idx < 0) + { res = false; + break; + } } PG_RETURN_BOOL(res);