[PATCH] Combine same ternary types in GIN and TSearch
Started by Pavel Borisovabout 5 years ago4 messages
Hi, hackers!
For historical reasons, now we have two differently named but similar
ternary data types in TSearch and Gin text-related types. Before v13 there
was also Gin's private TS_execute() version, from which we eventually
shifted to Tsearch's TS_execute().
To make things more even and beautiful I've made a minor refactor to
combine two left ternary types into one.
<gin.h>
typedef char GinTernaryValue
#define GIN_FALSE 0
#define GIN_TRUE 1
#define GIN_MAYBE 2
<ts_utils.h>
typedef enum { TS_NO, TS_YES, TS_MAYBE } TSTernaryValue;
The change is simple and most of it is just the text replacement. The only
thing worth noting is that some code does pointer cast between *bool and
*TernaryValue so the size of them should coincide. (Declaration done in
*char* type because simple enum on most architectures will be of *int*
size). There is no actual change in the code despite the order of header
files inclusion in some modules.
What do you think about this?
--
Best regards,
Pavel Borisov
Postgres Professional: http://postgrespro.com <http://www.postgrespro.com>
Attachments:
v1-0001-Combine-same-types-GinTernaryValue-and-TSTernaryV.patchapplication/octet-stream; name=v1-0001-Combine-same-types-GinTernaryValue-and-TSTernaryV.patchDownload
From 8e979bf6c33905cc3f79eb5e0b9ac41cf3d6cbca Mon Sep 17 00:00:00 2001
From: Pavel Borisov <pashkin.elfe@gmail.com>
Date: Thu, 12 Nov 2020 15:57:09 +0400
Subject: [PATCH v1] Combine same types GinTernaryValue and TSTernaryValue into
one
From past we had two types with different declarations and names but
similar meanings. Combine them into one to avoid confusion and make
things clear.
---
contrib/pg_trgm/trgm_gin.c | 37 ++++++-----
doc/src/sgml/gin.sgml | 24 +++----
src/backend/access/gin/ginarrayproc.c | 36 +++++-----
src/backend/access/gin/ginget.c | 34 +++++-----
src/backend/access/gin/ginlogic.c | 40 +++++------
src/backend/access/gin/ginscan.c | 2 +-
src/backend/utils/adt/jsonb_gin.c | 95 ++++++++++++++-------------
src/backend/utils/adt/tsginidx.c | 26 +++-----
src/include/access/gin.h | 16 ++---
src/include/access/gin_private.h | 5 +-
src/include/tsearch/ts_utils.h | 16 +++--
11 files changed, 161 insertions(+), 170 deletions(-)
diff --git a/contrib/pg_trgm/trgm_gin.c b/contrib/pg_trgm/trgm_gin.c
index 4dbf0ffb68..065ac7fd95 100644
--- a/contrib/pg_trgm/trgm_gin.c
+++ b/contrib/pg_trgm/trgm_gin.c
@@ -7,6 +7,7 @@
#include "access/stratnum.h"
#include "fmgr.h"
#include "trgm.h"
+#include "tsearch/ts_utils.h"
PG_FUNCTION_INFO_V1(gin_extract_trgm);
PG_FUNCTION_INFO_V1(gin_extract_value_trgm);
@@ -257,21 +258,21 @@ gin_trgm_consistent(PG_FUNCTION_ARGS)
}
/*
- * In all cases, GIN_TRUE is at least as favorable to inclusion as
- * GIN_MAYBE. If no better option is available, simply treat
- * GIN_MAYBE as if it were GIN_TRUE and apply the same test as the binary
+ * In all cases, TS_YES is at least as favorable to inclusion as
+ * TS_MAYBE. If no better option is available, simply treat
+ * TS_MAYBE as if it were TS_YES and apply the same test as the binary
* consistent function.
*/
Datum
gin_trgm_triconsistent(PG_FUNCTION_ARGS)
{
- GinTernaryValue *check = (GinTernaryValue *) PG_GETARG_POINTER(0);
+ TSTernaryValue *check = (TSTernaryValue *) PG_GETARG_POINTER(0);
StrategyNumber strategy = PG_GETARG_UINT16(1);
/* text *query = PG_GETARG_TEXT_PP(2); */
int32 nkeys = PG_GETARG_INT32(3);
Pointer *extra_data = (Pointer *) PG_GETARG_POINTER(4);
- GinTernaryValue res = GIN_MAYBE;
+ TSTernaryValue res = TS_MAYBE;
int32 i,
ntrue;
bool *boolcheck;
@@ -288,7 +289,7 @@ gin_trgm_triconsistent(PG_FUNCTION_ARGS)
ntrue = 0;
for (i = 0; i < nkeys; i++)
{
- if (check[i] != GIN_FALSE)
+ if (check[i] != TS_NO)
ntrue++;
}
@@ -297,8 +298,8 @@ gin_trgm_triconsistent(PG_FUNCTION_ARGS)
* formula
*/
res = (nkeys == 0)
- ? GIN_FALSE : (((((float4) ntrue) / ((float4) nkeys)) >= nlimit)
- ? GIN_MAYBE : GIN_FALSE);
+ ? TS_NO : (((((float4) ntrue) / ((float4) nkeys)) >= nlimit)
+ ? TS_MAYBE : TS_NO);
break;
case ILikeStrategyNumber:
#ifndef IGNORECASE
@@ -307,12 +308,12 @@ gin_trgm_triconsistent(PG_FUNCTION_ARGS)
/* FALL THRU */
case LikeStrategyNumber:
/* Check if all extracted trigrams are presented. */
- res = GIN_MAYBE;
+ res = TS_MAYBE;
for (i = 0; i < nkeys; i++)
{
- if (check[i] == GIN_FALSE)
+ if (check[i] == TS_NO)
{
- res = GIN_FALSE;
+ res = TS_NO;
break;
}
}
@@ -326,31 +327,31 @@ gin_trgm_triconsistent(PG_FUNCTION_ARGS)
if (nkeys < 1)
{
/* Regex processing gave no result: do full index scan */
- res = GIN_MAYBE;
+ res = TS_MAYBE;
}
else
{
/*
* As trigramsMatchGraph implements a monotonic boolean
- * function, promoting all GIN_MAYBE keys to GIN_TRUE will
+ * function, promoting all TS_MAYBE keys to TS_YES will
* give a conservative result.
*/
boolcheck = (bool *) palloc(sizeof(bool) * nkeys);
for (i = 0; i < nkeys; i++)
- boolcheck[i] = (check[i] != GIN_FALSE);
+ boolcheck[i] = (check[i] != TS_NO);
if (!trigramsMatchGraph((TrgmPackedGraph *) extra_data[0],
boolcheck))
- res = GIN_FALSE;
+ res = TS_NO;
pfree(boolcheck);
}
break;
default:
elog(ERROR, "unrecognized strategy number: %d", strategy);
- res = GIN_FALSE; /* keep compiler quiet */
+ res = TS_NO; /* keep compiler quiet */
break;
}
/* All cases served by this function are inexact */
- Assert(res != GIN_TRUE);
- PG_RETURN_GIN_TERNARY_VALUE(res);
+ Assert(res != TS_YES);
+ PG_RETURN_TERNARY_VALUE(res);
}
diff --git a/doc/src/sgml/gin.sgml b/doc/src/sgml/gin.sgml
index 67754f52f6..fa0329ba0b 100644
--- a/doc/src/sgml/gin.sgml
+++ b/doc/src/sgml/gin.sgml
@@ -326,7 +326,7 @@
</varlistentry>
<varlistentry>
- <term><function>GinTernaryValue triConsistent(GinTernaryValue check[], StrategyNumber n, Datum query,
+ <term><function>TSTernaryValue triConsistent(TSTernaryValue check[], StrategyNumber n, Datum query,
int32 nkeys, Pointer extra_data[],
Datum queryKeys[], bool nullFlags[])</function></term>
<listitem>
@@ -334,22 +334,22 @@
<function>triConsistent</function> is similar to <function>consistent</function>,
but instead of Booleans in the <literal>check</literal> vector, there are
three possible values for each
- key: <literal>GIN_TRUE</literal>, <literal>GIN_FALSE</literal> and
- <literal>GIN_MAYBE</literal>. <literal>GIN_FALSE</literal> and <literal>GIN_TRUE</literal>
+ key: <literal>TS_YES</literal>, <literal>TS_NO</literal> and
+ <literal>TS_MAYBE</literal>. <literal>TS_NO</literal> and <literal>TS_YES</literal>
have the same meaning as regular Boolean values, while
- <literal>GIN_MAYBE</literal> means that the presence of that key is not known.
- When <literal>GIN_MAYBE</literal> values are present, the function should only
- return <literal>GIN_TRUE</literal> if the item certainly matches whether or
+ <literal>TS_MAYBE</literal> means that the presence of that key is not known.
+ When <literal>TS_MAYBE</literal> values are present, the function should only
+ return <literal>TS_YES</literal> if the item certainly matches whether or
not the index item contains the corresponding query keys. Likewise, the
- function must return <literal>GIN_FALSE</literal> only if the item certainly
- does not match, whether or not it contains the <literal>GIN_MAYBE</literal>
- keys. If the result depends on the <literal>GIN_MAYBE</literal> entries, i.e.,
+ function must return <literal>TS_NO</literal> only if the item certainly
+ does not match, whether or not it contains the <literal>TS_MAYBE</literal>
+ keys. If the result depends on the <literal>TS_MAYBE</literal> entries, i.e.,
the match cannot be confirmed or refuted based on the known query keys,
- the function must return <literal>GIN_MAYBE</literal>.
+ the function must return <literal>TS_MAYBE</literal>.
</para>
<para>
- When there are no <literal>GIN_MAYBE</literal> values in the <literal>check</literal>
- vector, a <literal>GIN_MAYBE</literal> return value is the equivalent of
+ When there are no <literal>TS_MAYBE</literal> values in the <literal>check</literal>
+ vector, a <literal>TS_MAYBE</literal> return value is the equivalent of
setting the <literal>recheck</literal> flag in the
Boolean <function>consistent</function> function.
</para>
diff --git a/src/backend/access/gin/ginarrayproc.c b/src/backend/access/gin/ginarrayproc.c
index 3a6d54b38c..366b1797c9 100644
--- a/src/backend/access/gin/ginarrayproc.c
+++ b/src/backend/access/gin/ginarrayproc.c
@@ -18,7 +18,7 @@
#include "utils/array.h"
#include "utils/builtins.h"
#include "utils/lsyscache.h"
-
+#include "tsearch/ts_utils.h"
#define GinOverlapStrategy 1
#define GinContainsStrategy 2
@@ -225,7 +225,7 @@ ginarrayconsistent(PG_FUNCTION_ARGS)
Datum
ginarraytriconsistent(PG_FUNCTION_ARGS)
{
- GinTernaryValue *check = (GinTernaryValue *) PG_GETARG_POINTER(0);
+ TSTernaryValue *check = (TSTernaryValue *) PG_GETARG_POINTER(0);
StrategyNumber strategy = PG_GETARG_UINT16(1);
/* ArrayType *query = PG_GETARG_ARRAYTYPE_P(2); */
@@ -234,49 +234,49 @@ ginarraytriconsistent(PG_FUNCTION_ARGS)
/* Pointer *extra_data = (Pointer *) PG_GETARG_POINTER(4); */
/* Datum *queryKeys = (Datum *) PG_GETARG_POINTER(5); */
bool *nullFlags = (bool *) PG_GETARG_POINTER(6);
- GinTernaryValue res;
+ TSTernaryValue res;
int32 i;
switch (strategy)
{
case GinOverlapStrategy:
/* must have a match for at least one non-null element */
- res = GIN_FALSE;
+ res = TS_NO;
for (i = 0; i < nkeys; i++)
{
if (!nullFlags[i])
{
- if (check[i] == GIN_TRUE)
+ if (check[i] == TS_YES)
{
- res = GIN_TRUE;
+ res = TS_YES;
break;
}
- else if (check[i] == GIN_MAYBE && res == GIN_FALSE)
+ else if (check[i] == TS_MAYBE && res == TS_NO)
{
- res = GIN_MAYBE;
+ res = TS_MAYBE;
}
}
}
break;
case GinContainsStrategy:
/* must have all elements in check[] true, and no nulls */
- res = GIN_TRUE;
+ res = TS_YES;
for (i = 0; i < nkeys; i++)
{
- if (check[i] == GIN_FALSE || nullFlags[i])
+ if (check[i] == TS_NO || nullFlags[i])
{
- res = GIN_FALSE;
+ res = TS_NO;
break;
}
- if (check[i] == GIN_MAYBE)
+ if (check[i] == TS_MAYBE)
{
- res = GIN_MAYBE;
+ res = TS_MAYBE;
}
}
break;
case GinContainedStrategy:
/* can't do anything else useful here */
- res = GIN_MAYBE;
+ res = TS_MAYBE;
break;
case GinEqualStrategy:
@@ -285,12 +285,12 @@ ginarraytriconsistent(PG_FUNCTION_ARGS)
* against nulls here. This is because array_contain_compare and
* array_eq handle nulls differently ...
*/
- res = GIN_MAYBE;
+ res = TS_MAYBE;
for (i = 0; i < nkeys; i++)
{
- if (check[i] == GIN_FALSE)
+ if (check[i] == TS_NO)
{
- res = GIN_FALSE;
+ res = TS_NO;
break;
}
}
@@ -301,5 +301,5 @@ ginarraytriconsistent(PG_FUNCTION_ARGS)
res = false;
}
- PG_RETURN_GIN_TERNARY_VALUE(res);
+ PG_RETURN_TERNARY_VALUE(res);
}
diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c
index 2cfccdedcf..2572dd4739 100644
--- a/src/backend/access/gin/ginget.c
+++ b/src/backend/access/gin/ginget.c
@@ -559,11 +559,11 @@ startScanKey(GinState *ginstate, GinScanOpaque so, GinScanKey key)
{
/* Pass all entries <= i as FALSE, and the rest as MAYBE */
for (j = 0; j <= i; j++)
- key->entryRes[entryIndexes[j]] = GIN_FALSE;
+ key->entryRes[entryIndexes[j]] = TS_NO;
for (j = i + 1; j < key->nentries; j++)
- key->entryRes[entryIndexes[j]] = GIN_MAYBE;
+ key->entryRes[entryIndexes[j]] = TS_MAYBE;
- if (key->triConsistentFn(key) == GIN_FALSE)
+ if (key->triConsistentFn(key) == TS_NO)
break;
}
/* i is now the last required entry. */
@@ -994,7 +994,7 @@ keyGetItem(GinState *ginstate, MemoryContext tempCtx, GinScanKey key,
uint32 i;
bool haveLossyEntry;
GinScanEntry entry;
- GinTernaryValue res;
+ TSTernaryValue res;
MemoryContext oldCtx;
bool allFinished;
@@ -1167,13 +1167,13 @@ keyGetItem(GinState *ginstate, MemoryContext tempCtx, GinScanKey key,
ginCompareItemPointers(&entry->curItem, &curPageLossy) == 0)
{
if (i < key->nuserentries)
- key->entryRes[i] = GIN_MAYBE;
+ key->entryRes[i] = TS_MAYBE;
else
- key->entryRes[i] = GIN_TRUE;
+ key->entryRes[i] = TS_YES;
haveLossyEntry = true;
}
else
- key->entryRes[i] = GIN_FALSE;
+ key->entryRes[i] = TS_NO;
}
/* prepare for calling consistentFn in temp context */
@@ -1184,7 +1184,7 @@ keyGetItem(GinState *ginstate, MemoryContext tempCtx, GinScanKey key,
/* Have lossy-page entries, so see if whole page matches */
res = key->triConsistentFn(key);
- if (res == GIN_TRUE || res == GIN_MAYBE)
+ if (res == TS_YES || res == TS_MAYBE)
{
/* Yes, so clean up ... */
MemoryContextSwitchTo(oldCtx);
@@ -1212,7 +1212,7 @@ keyGetItem(GinState *ginstate, MemoryContext tempCtx, GinScanKey key,
{
entry = key->scanEntry[i];
if (entry->isFinished)
- key->entryRes[i] = GIN_FALSE;
+ key->entryRes[i] = TS_NO;
#if 0
/*
@@ -1220,30 +1220,30 @@ keyGetItem(GinState *ginstate, MemoryContext tempCtx, GinScanKey key,
* for this item earlier.
*/
else if (ginCompareItemPointers(&entry->curItem, &advancePast) <= 0)
- key->entryRes[i] = GIN_MAYBE;
+ key->entryRes[i] = TS_MAYBE;
#endif
else if (ginCompareItemPointers(&entry->curItem, &curPageLossy) == 0)
- key->entryRes[i] = GIN_MAYBE;
+ key->entryRes[i] = TS_MAYBE;
else if (ginCompareItemPointers(&entry->curItem, &minItem) == 0)
- key->entryRes[i] = GIN_TRUE;
+ key->entryRes[i] = TS_YES;
else
- key->entryRes[i] = GIN_FALSE;
+ key->entryRes[i] = TS_NO;
}
res = key->triConsistentFn(key);
switch (res)
{
- case GIN_TRUE:
+ case TS_YES:
key->curItemMatches = true;
/* triConsistentFn set recheckCurItem */
break;
- case GIN_FALSE:
+ case TS_NO:
key->curItemMatches = false;
break;
- case GIN_MAYBE:
+ case TS_MAYBE:
key->curItemMatches = true;
key->recheckCurItem = true;
break;
@@ -1619,7 +1619,7 @@ collectMatchesForHeapRow(IndexScanDesc scan, pendingPosition *pos)
{
GinScanKey key = so->keys + i;
- memset(key->entryRes, GIN_FALSE, key->nentries);
+ memset(key->entryRes, TS_NO, key->nentries);
}
memset(pos->hasMatchKey, false, so->nkeys);
diff --git a/src/backend/access/gin/ginlogic.c b/src/backend/access/gin/ginlogic.c
index bcbc26efdb..16a7440043 100644
--- a/src/backend/access/gin/ginlogic.c
+++ b/src/backend/access/gin/ginlogic.c
@@ -10,7 +10,7 @@
*
* Providing a boolean interface when the opclass implements only the
* ternary function is straightforward - just call the ternary function
- * with the check-array as is, and map the GIN_TRUE, GIN_FALSE, GIN_MAYBE
+ * with the check-array as is, and map the TS_YES, TS_NO, TS_MAYBE
* return codes to TRUE, FALSE and TRUE+recheck, respectively. Providing
* a ternary interface when the opclass only implements a boolean function
* is implemented by calling the boolean function many times, with all the
@@ -58,10 +58,10 @@ trueConsistentFn(GinScanKey key)
key->recheckCurItem = false;
return true;
}
-static GinTernaryValue
+static TSTernaryValue
trueTriConsistentFn(GinScanKey key)
{
- return GIN_TRUE;
+ return TS_YES;
}
/*
@@ -91,10 +91,10 @@ directBoolConsistentFn(GinScanKey key)
/*
* A helper function for calling a native ternary logic consistent function.
*/
-static GinTernaryValue
+static TSTernaryValue
directTriConsistentFn(GinScanKey key)
{
- return DatumGetGinTernaryValue(FunctionCall7Coll(key->triConsistentFmgrInfo,
+ return DatumGetTSTernaryValue(FunctionCall7Coll(key->triConsistentFmgrInfo,
key->collation,
PointerGetDatum(key->entryRes),
UInt16GetDatum(key->strategy),
@@ -107,15 +107,15 @@ directTriConsistentFn(GinScanKey key)
/*
* This function implements a binary logic consistency check, using a ternary
- * logic consistent function provided by the opclass. GIN_MAYBE return value
+ * logic consistent function provided by the opclass. TS_MAYBE return value
* is interpreted as true with recheck flag.
*/
static bool
shimBoolConsistentFn(GinScanKey key)
{
- GinTernaryValue result;
+ TSTernaryValue result;
- result = DatumGetGinTernaryValue(FunctionCall7Coll(key->triConsistentFmgrInfo,
+ result = DatumGetTSTernaryValue(FunctionCall7Coll(key->triConsistentFmgrInfo,
key->collation,
PointerGetDatum(key->entryRes),
UInt16GetDatum(key->strategy),
@@ -124,7 +124,7 @@ shimBoolConsistentFn(GinScanKey key)
PointerGetDatum(key->extra_data),
PointerGetDatum(key->queryValues),
PointerGetDatum(key->queryCategories)));
- if (result == GIN_MAYBE)
+ if (result == TS_MAYBE)
{
key->recheckCurItem = true;
return true;
@@ -148,7 +148,7 @@ shimBoolConsistentFn(GinScanKey key)
*
* NB: This function modifies the key->entryRes array!
*/
-static GinTernaryValue
+static TSTernaryValue
shimTriConsistentFn(GinScanKey key)
{
int nmaybe;
@@ -156,7 +156,7 @@ shimTriConsistentFn(GinScanKey key)
int i;
bool boolResult;
bool recheck = false;
- GinTernaryValue curResult;
+ TSTernaryValue curResult;
/*
* Count how many MAYBE inputs there are, and store their indexes in
@@ -166,10 +166,10 @@ shimTriConsistentFn(GinScanKey key)
nmaybe = 0;
for (i = 0; i < key->nentries; i++)
{
- if (key->entryRes[i] == GIN_MAYBE)
+ if (key->entryRes[i] == TS_MAYBE)
{
if (nmaybe >= MAX_MAYBE_ENTRIES)
- return GIN_MAYBE;
+ return TS_MAYBE;
maybeEntries[nmaybe++] = i;
}
}
@@ -183,7 +183,7 @@ shimTriConsistentFn(GinScanKey key)
/* First call consistent function with all the maybe-inputs set FALSE */
for (i = 0; i < nmaybe; i++)
- key->entryRes[maybeEntries[i]] = GIN_FALSE;
+ key->entryRes[maybeEntries[i]] = TS_NO;
curResult = directBoolConsistentFn(key);
for (;;)
@@ -191,13 +191,13 @@ shimTriConsistentFn(GinScanKey key)
/* Twiddle the entries for next combination. */
for (i = 0; i < nmaybe; i++)
{
- if (key->entryRes[maybeEntries[i]] == GIN_FALSE)
+ if (key->entryRes[maybeEntries[i]] == TS_NO)
{
- key->entryRes[maybeEntries[i]] = GIN_TRUE;
+ key->entryRes[maybeEntries[i]] = TS_YES;
break;
}
else
- key->entryRes[maybeEntries[i]] = GIN_FALSE;
+ key->entryRes[maybeEntries[i]] = TS_NO;
}
if (i == nmaybe)
break;
@@ -206,12 +206,12 @@ shimTriConsistentFn(GinScanKey key)
recheck |= key->recheckCurItem;
if (curResult != boolResult)
- return GIN_MAYBE;
+ return TS_MAYBE;
}
/* TRUE with recheck is taken to mean MAYBE */
- if (curResult == GIN_TRUE && recheck)
- curResult = GIN_MAYBE;
+ if (curResult == TS_YES && recheck)
+ curResult = TS_MAYBE;
return curResult;
}
diff --git a/src/backend/access/gin/ginscan.c b/src/backend/access/gin/ginscan.c
index 0a685bdbfc..05252bc631 100644
--- a/src/backend/access/gin/ginscan.c
+++ b/src/backend/access/gin/ginscan.c
@@ -166,7 +166,7 @@ ginFillScanKey(GinScanOpaque so, OffsetNumber attnum,
/* Allocate one extra array slot for possible "hidden" entry */
key->scanEntry = (GinScanEntry *) palloc(sizeof(GinScanEntry) *
(nQueryValues + 1));
- key->entryRes = (GinTernaryValue *) palloc0(sizeof(GinTernaryValue) *
+ key->entryRes = (TSTernaryValue *) palloc0(sizeof(TSTernaryValue) *
(nQueryValues + 1));
key->query = query;
diff --git a/src/backend/utils/adt/jsonb_gin.c b/src/backend/utils/adt/jsonb_gin.c
index aee3d9d673..a852b3e0fb 100644
--- a/src/backend/utils/adt/jsonb_gin.c
+++ b/src/backend/utils/adt/jsonb_gin.c
@@ -69,6 +69,7 @@
#include "utils/jsonb.h"
#include "utils/jsonpath.h"
#include "utils/varlena.h"
+#include "tsearch/ts_utils.h"
typedef struct PathHashStack
{
@@ -428,7 +429,7 @@ jsonb_ops__extract_nodes(JsonPathGinContext *cxt, JsonPathGinPath path,
if (scalar->type == jbvString)
{
JsonPathGinPathItem *last = path.items;
- GinTernaryValue key_entry;
+ TSTernaryValue key_entry;
/*
* Assuming that jsonb_ops interprets string array elements as
@@ -439,17 +440,17 @@ jsonb_ops__extract_nodes(JsonPathGinContext *cxt, JsonPathGinPath path,
*/
if (cxt->lax)
- key_entry = GIN_MAYBE;
+ key_entry = TS_MAYBE;
else if (!last) /* root ($) */
- key_entry = GIN_FALSE;
+ key_entry = TS_NO;
else if (last->type == jpiAnyArray || last->type == jpiIndexArray)
- key_entry = GIN_TRUE;
+ key_entry = TS_YES;
else if (last->type == jpiAny)
- key_entry = GIN_MAYBE;
+ key_entry = TS_MAYBE;
else
- key_entry = GIN_FALSE;
+ key_entry = TS_NO;
- if (key_entry == GIN_MAYBE)
+ if (key_entry == TS_MAYBE)
{
JsonPathGinNode *n1 = make_jsp_entry_node_scalar(scalar, true);
JsonPathGinNode *n2 = make_jsp_entry_node_scalar(scalar, false);
@@ -459,7 +460,7 @@ jsonb_ops__extract_nodes(JsonPathGinContext *cxt, JsonPathGinPath path,
else
{
node = make_jsp_entry_node_scalar(scalar,
- key_entry == GIN_TRUE);
+ key_entry == TS_YES);
}
}
else
@@ -793,38 +794,38 @@ extract_jsp_query(JsonPath *jp, StrategyNumber strat, bool pathOps,
/*
* Recursively execute jsonpath expression.
- * 'check' is a bool[] or a GinTernaryValue[] depending on 'ternary' flag.
+ * 'check' is a bool[] or a TSTernaryValue[] depending on 'ternary' flag.
*/
-static GinTernaryValue
+static TSTernaryValue
execute_jsp_gin_node(JsonPathGinNode *node, void *check, bool ternary)
{
- GinTernaryValue res;
- GinTernaryValue v;
+ TSTernaryValue res;
+ TSTernaryValue v;
int i;
switch (node->type)
{
case JSP_GIN_AND:
- res = GIN_TRUE;
+ res = TS_YES;
for (i = 0; i < node->val.nargs; i++)
{
v = execute_jsp_gin_node(node->args[i], check, ternary);
- if (v == GIN_FALSE)
- return GIN_FALSE;
- else if (v == GIN_MAYBE)
- res = GIN_MAYBE;
+ if (v == TS_NO)
+ return TS_NO;
+ else if (v == TS_MAYBE)
+ res = TS_MAYBE;
}
return res;
case JSP_GIN_OR:
- res = GIN_FALSE;
+ res = TS_NO;
for (i = 0; i < node->val.nargs; i++)
{
v = execute_jsp_gin_node(node->args[i], check, ternary);
- if (v == GIN_TRUE)
- return GIN_TRUE;
- else if (v == GIN_MAYBE)
- res = GIN_MAYBE;
+ if (v == TS_YES)
+ return TS_YES;
+ else if (v == TS_MAYBE)
+ res = TS_MAYBE;
}
return res;
@@ -833,14 +834,14 @@ execute_jsp_gin_node(JsonPathGinNode *node, void *check, bool ternary)
int index = node->val.entryIndex;
if (ternary)
- return ((GinTernaryValue *) check)[index];
+ return ((TSTernaryValue *) check)[index];
else
- return ((bool *) check)[index] ? GIN_TRUE : GIN_FALSE;
+ return ((bool *) check)[index] ? TS_YES : TS_NO;
}
default:
elog(ERROR, "invalid jsonpath gin node type: %d", node->type);
- return GIN_FALSE; /* keep compiler quiet */
+ return TS_NO; /* keep compiler quiet */
}
}
@@ -1001,7 +1002,7 @@ gin_consistent_jsonb(PG_FUNCTION_ARGS)
{
Assert(extra_data && extra_data[0]);
res = execute_jsp_gin_node((JsonPathGinNode *) extra_data[0], check,
- false) != GIN_FALSE;
+ false) != TS_NO;
}
}
else
@@ -1013,17 +1014,17 @@ gin_consistent_jsonb(PG_FUNCTION_ARGS)
Datum
gin_triconsistent_jsonb(PG_FUNCTION_ARGS)
{
- GinTernaryValue *check = (GinTernaryValue *) PG_GETARG_POINTER(0);
+ TSTernaryValue *check = (TSTernaryValue *) PG_GETARG_POINTER(0);
StrategyNumber strategy = PG_GETARG_UINT16(1);
/* Jsonb *query = PG_GETARG_JSONB_P(2); */
int32 nkeys = PG_GETARG_INT32(3);
Pointer *extra_data = (Pointer *) PG_GETARG_POINTER(4);
- GinTernaryValue res = GIN_MAYBE;
+ TSTernaryValue res = TS_MAYBE;
int32 i;
/*
- * Note that we never return GIN_TRUE, only GIN_MAYBE or GIN_FALSE; this
+ * Note that we never return TS_YES, only TS_MAYBE or TS_NO; this
* corresponds to always forcing recheck in the regular consistent
* function, for the reasons listed there.
*/
@@ -1033,9 +1034,9 @@ gin_triconsistent_jsonb(PG_FUNCTION_ARGS)
/* All extracted keys must be present */
for (i = 0; i < nkeys; i++)
{
- if (check[i] == GIN_FALSE)
+ if (check[i] == TS_NO)
{
- res = GIN_FALSE;
+ res = TS_NO;
break;
}
}
@@ -1044,13 +1045,13 @@ gin_triconsistent_jsonb(PG_FUNCTION_ARGS)
strategy == JsonbExistsAnyStrategyNumber)
{
/* At least one extracted key must be present */
- res = GIN_FALSE;
+ res = TS_NO;
for (i = 0; i < nkeys; i++)
{
- if (check[i] == GIN_TRUE ||
- check[i] == GIN_MAYBE)
+ if (check[i] == TS_YES ||
+ check[i] == TS_MAYBE)
{
- res = GIN_MAYBE;
+ res = TS_MAYBE;
break;
}
}
@@ -1065,14 +1066,14 @@ gin_triconsistent_jsonb(PG_FUNCTION_ARGS)
true);
/* Should always recheck the result */
- if (res == GIN_TRUE)
- res = GIN_MAYBE;
+ if (res == TS_YES)
+ res = TS_MAYBE;
}
}
else
elog(ERROR, "unrecognized strategy number: %d", strategy);
- PG_RETURN_GIN_TERNARY_VALUE(res);
+ PG_RETURN_TERNARY_VALUE(res);
}
/*
@@ -1260,7 +1261,7 @@ gin_consistent_jsonb_path(PG_FUNCTION_ARGS)
{
Assert(extra_data && extra_data[0]);
res = execute_jsp_gin_node((JsonPathGinNode *) extra_data[0], check,
- false) != GIN_FALSE;
+ false) != TS_NO;
}
}
else
@@ -1272,27 +1273,27 @@ gin_consistent_jsonb_path(PG_FUNCTION_ARGS)
Datum
gin_triconsistent_jsonb_path(PG_FUNCTION_ARGS)
{
- GinTernaryValue *check = (GinTernaryValue *) PG_GETARG_POINTER(0);
+ TSTernaryValue *check = (TSTernaryValue *) PG_GETARG_POINTER(0);
StrategyNumber strategy = PG_GETARG_UINT16(1);
/* Jsonb *query = PG_GETARG_JSONB_P(2); */
int32 nkeys = PG_GETARG_INT32(3);
Pointer *extra_data = (Pointer *) PG_GETARG_POINTER(4);
- GinTernaryValue res = GIN_MAYBE;
+ TSTernaryValue res = TS_MAYBE;
int32 i;
if (strategy == JsonbContainsStrategyNumber)
{
/*
- * Note that we never return GIN_TRUE, only GIN_MAYBE or GIN_FALSE;
+ * Note that we never return TS_YES, only TS_MAYBE or TS_NO;
* this corresponds to always forcing recheck in the regular
* consistent function, for the reasons listed there.
*/
for (i = 0; i < nkeys; i++)
{
- if (check[i] == GIN_FALSE)
+ if (check[i] == TS_NO)
{
- res = GIN_FALSE;
+ res = TS_NO;
break;
}
}
@@ -1307,14 +1308,14 @@ gin_triconsistent_jsonb_path(PG_FUNCTION_ARGS)
true);
/* Should always recheck the result */
- if (res == GIN_TRUE)
- res = GIN_MAYBE;
+ if (res == TS_YES)
+ res = TS_MAYBE;
}
}
else
elog(ERROR, "unrecognized strategy number: %d", strategy);
- PG_RETURN_GIN_TERNARY_VALUE(res);
+ PG_RETURN_TERNARY_VALUE(res);
}
/*
diff --git a/src/backend/utils/adt/tsginidx.c b/src/backend/utils/adt/tsginidx.c
index b3e3ffc577..87f64b1a9e 100644
--- a/src/backend/utils/adt/tsginidx.c
+++ b/src/backend/utils/adt/tsginidx.c
@@ -173,7 +173,7 @@ gin_extract_tsquery(PG_FUNCTION_ARGS)
typedef struct
{
QueryItem *first_item;
- GinTernaryValue *check;
+ TSTernaryValue *check;
int *map_item_operand;
bool *need_recheck;
} GinChkVal;
@@ -201,18 +201,13 @@ checkcondition_gin(void *checkval, QueryOperand *val, ExecPhraseData *data)
* return presence of current entry in indexed value; but TRUE becomes
* MAYBE in the presence of a query requiring recheck
*/
- if (gcv->check[j] == GIN_TRUE)
+ if (gcv->check[j] == TS_YES)
{
if (val->weight != 0 || data != NULL)
return TS_MAYBE;
}
- /*
- * We rely on GinTernaryValue and TSTernaryValue using equivalent value
- * assignments. We could use a switch statement to map the values if that
- * ever stops being true, but it seems unlikely to happen.
- */
- return (TSTernaryValue) gcv->check[j];
+ return gcv->check[j];
}
Datum
@@ -234,15 +229,14 @@ gin_tsquery_consistent(PG_FUNCTION_ARGS)
if (query->size > 0)
{
GinChkVal gcv;
-
/*
* check-parameter array has one entry for each value (operand) in the
* query.
*/
gcv.first_item = GETQUERY(query);
- StaticAssertStmt(sizeof(GinTernaryValue) == sizeof(bool),
- "sizes of GinTernaryValue and bool are not equal");
- gcv.check = (GinTernaryValue *) check;
+ StaticAssertStmt(sizeof(TSTernaryValue) == sizeof(bool),
+ "sizes of TSTernaryValue and bool are not equal");
+ gcv.check = (TSTernaryValue *) check;
gcv.map_item_operand = (int *) (extra_data[0]);
gcv.need_recheck = recheck;
@@ -258,14 +252,14 @@ gin_tsquery_consistent(PG_FUNCTION_ARGS)
Datum
gin_tsquery_triconsistent(PG_FUNCTION_ARGS)
{
- GinTernaryValue *check = (GinTernaryValue *) PG_GETARG_POINTER(0);
+ TSTernaryValue *check = (TSTernaryValue *) PG_GETARG_POINTER(0);
/* StrategyNumber strategy = PG_GETARG_UINT16(1); */
TSQuery query = PG_GETARG_TSQUERY(2);
/* int32 nkeys = PG_GETARG_INT32(3); */
Pointer *extra_data = (Pointer *) PG_GETARG_POINTER(4);
- GinTernaryValue res = GIN_FALSE;
+ TSTernaryValue res = TS_NO;
bool recheck;
/* Initially assume query doesn't require recheck */
@@ -288,10 +282,10 @@ gin_tsquery_triconsistent(PG_FUNCTION_ARGS)
&gcv,
TS_EXEC_PHRASE_NO_POS,
checkcondition_gin))
- res = recheck ? GIN_MAYBE : GIN_TRUE;
+ res = recheck ? TS_MAYBE : TS_YES;
}
- PG_RETURN_GIN_TERNARY_VALUE(res);
+ PG_RETURN_TERNARY_VALUE(res);
}
/*
diff --git a/src/include/access/gin.h b/src/include/access/gin.h
index 990e8b3e4f..6874fd69e5 100644
--- a/src/include/access/gin.h
+++ b/src/include/access/gin.h
@@ -15,7 +15,6 @@
#include "storage/block.h"
#include "utils/relcache.h"
-
/*
* amproc indexes for inverted indexes.
*/
@@ -53,18 +52,11 @@ typedef struct GinStatsData
* A ternary value used by tri-consistent functions.
*
* This must be of the same size as a bool because some code will cast a
- * pointer to a bool to a pointer to a GinTernaryValue.
+ * pointer to a bool to a pointer to a TSTernaryValue.
*/
-typedef char GinTernaryValue;
-
-#define GIN_FALSE 0 /* item is not present / does not match */
-#define GIN_TRUE 1 /* item is present / matches */
-#define GIN_MAYBE 2 /* don't know if item is present / don't know
- * if matches */
-
-#define DatumGetGinTernaryValue(X) ((GinTernaryValue)(X))
-#define GinTernaryValueGetDatum(X) ((Datum)(X))
-#define PG_RETURN_GIN_TERNARY_VALUE(x) return GinTernaryValueGetDatum(x)
+#define DatumGetTSTernaryValue(X) ((TSTernaryValue)(X))
+#define TSTernaryValueGetDatum(X) ((Datum)(X))
+#define PG_RETURN_TERNARY_VALUE(x) return TSTernaryValueGetDatum(x)
/* GUC parameters */
extern PGDLLIMPORT int GinFuzzySearchLimit;
diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h
index 5cb2f72e4c..664b15116f 100644
--- a/src/include/access/gin_private.h
+++ b/src/include/access/gin_private.h
@@ -18,6 +18,7 @@
#include "fmgr.h"
#include "lib/rbtree.h"
#include "storage/bufmgr.h"
+#include "tsearch/ts_utils.h"
/*
* Storage type for GIN's reloptions
@@ -286,9 +287,9 @@ typedef struct GinScanKeyData
int nadditional;
/* array of check flags, reported to consistentFn */
- GinTernaryValue *entryRes;
+ TSTernaryValue *entryRes;
bool (*boolConsistentFn) (GinScanKey key);
- GinTernaryValue (*triConsistentFn) (GinScanKey key);
+ TSTernaryValue (*triConsistentFn) (GinScanKey key);
FmgrInfo *consistentFmgrInfo;
FmgrInfo *triConsistentFmgrInfo;
Oid collation;
diff --git a/src/include/tsearch/ts_utils.h b/src/include/tsearch/ts_utils.h
index 400ba33001..37cb9f0ce5 100644
--- a/src/include/tsearch/ts_utils.h
+++ b/src/include/tsearch/ts_utils.h
@@ -124,13 +124,15 @@ extern text *generateHeadline(HeadlineParsedText *prs);
* whether a given primitive tsquery value is matched in the data.
*/
-/* TS_execute requires ternary logic to handle NOT with phrase matches */
-typedef enum
-{
- TS_NO, /* definitely no match */
- TS_YES, /* definitely does match */
- TS_MAYBE /* can't verify match for lack of pos data */
-} TSTernaryValue;
+/* TS_execute requires ternary logic to handle NOT with phrase matches.
+ * This must be of the same size as a bool because some code will cast a
+ * pointer to a bool to a pointer to a TSTernaryValue
+ */
+typedef char TSTernaryValue;
+
+#define TS_NO 0 /* definitely no match */
+#define TS_YES 1 /* definitely does match */
+#define TS_MAYBE 2 /* can't verify match for lack of pos data */
/*
* struct ExecPhraseData is passed to a TSExecuteCallback function if we need
--
2.28.0
Re: [PATCH] Combine same ternary types in GIN and TSearch
On 13/11/2020 11:04, Pavel Borisov wrote:
Hi, hackers!
For historical reasons, now we have two differently named but similar
ternary data types in TSearch and Gin text-related types. Before v13
there was also Gin's private TS_execute() version, from which we
eventually shifted to Tsearch's TS_execute().To make things more even and beautiful I've made a minor refactor to
combine two left ternary types into one.<gin.h>
typedef char GinTernaryValue
#define GIN_FALSE 0
#define GIN_TRUE 1
#define GIN_MAYBE 2<ts_utils.h>
typedef enum { TS_NO, TS_YES, TS_MAYBE } TSTernaryValue;The change is simple and most of it is just the text replacement. The
only thing worth noting is that some code does pointer cast between
*bool and *TernaryValue so the size of them should coincide.
(Declaration done in /char/ type because simple enum on most
architectures will be of /int/ size). There is no actual change in the
code despite the order of header files inclusion in some modules.What do you think about this?
GIN is not just for full-text search, so using TSTernaryValue in
GinScanKeyData is wrong. And it would break existing extensions.
I didn't look much further than that, but I've got a feeling that
combining those is a bad idea. TSTernaryValue is used in text-search
code, even when there is no GIN involved. It's a separate concept, even
though it happens to have the same values.
- Heikki
Re: [PATCH] Combine same ternary types in GIN and TSearch
GIN is not just for full-text search, so using TSTernaryValue in
GinScanKeyData is wrong. And it would break existing extensions.I didn't look much further than that, but I've got a feeling that
combining those is a bad idea. TSTernaryValue is used in text-search
code, even when there is no GIN involved. It's a separate concept, even
though it happens to have the same values.
Probably you are right. But now the code already rely on equivalent value
assignments for GinTernaryValue and TSTernaryValue
(in checkcondition_gin()). So my idea was to combine them and use them like
we use other global data types. We may declare it somewhere outside both
gin and search. Or just leave as it is.
Thank you, Heikki for your feedback!
--
Best regards,
Pavel Borisov
Postgres Professional: http://postgrespro.com <http://www.postgrespro.com>
Re: [PATCH] Combine same ternary types in GIN and TSearch
Heikki Linnakangas <hlinnaka@iki.fi> writes:
On 13/11/2020 11:04, Pavel Borisov wrote:
For historical reasons, now we have two differently named but similar
ternary data types in TSearch and Gin text-related types. Before v13
there was also Gin's private TS_execute() version, from which we
eventually shifted to Tsearch's TS_execute().
To make things more even and beautiful I've made a minor refactor to
combine two left ternary types into one.
GIN is not just for full-text search, so using TSTernaryValue in
GinScanKeyData is wrong. And it would break existing extensions.
I didn't look much further than that, but I've got a feeling that
combining those is a bad idea. TSTernaryValue is used in text-search
code, even when there is no GIN involved. It's a separate concept, even
though it happens to have the same values.
I'm definitely not on board with importing a TS-specific type into GIN,
and even less with requiring major GIN headers to import random
TS-related headers.
There might be a case for having just one neutrally-named "ternary" enum
type, declared in a neutral (probably new) header, that both areas of
the code could use. But it's not clear that it'd be worth the code
thrashing to do that. As Heikki says, this will surely break some
extensions; and I'd prefer that there be some non-cosmetic benefit
if we ask extension authors to cope with that.
regards, tom lane