Replace TS_execute's TS_EXEC_CALC_NOT flag with TS_EXEC_SKIP_NOT.

It's fairly silly that ignoring NOT subexpressions is TS_execute's
default behavior.  It's wrong on its face and it encourages errors
of omission.  Moreover, the only two remaining callers that aren't
specifying CALC_NOT are in ts_headline calculations, and it's very
arguable that those are bugs: if you've specified "!foo" in your
query, why would you want to get a headline that includes "foo"?

Hence, rip that out and change the default behavior to be to calculate
NOT accurately.  As a concession to the slim chance that there is still
somebody somewhere who needs the incorrect behavior, provide a new
SKIP_NOT flag to explicitly request that.

Back-patch into v13, mainly because it seems better to change this
at the same time as the previous commit's rejiggering of TS_execute
related APIs.  Any outside callers affected by this change are
probably also affected by that one.

Discussion: https://postgr.es/m/CALT9ZEE-aLotzBg-pOp2GFTesGWVYzXA3=mZKzRDa_OKnLF7Mg@mail.gmail.com

diff --git a/src/backend/utils/adt/tsginidx.c b/src/backend/utils/adt/tsginidx.c
index 3128f0a7da074174630512d77d3a4b8fca96e909..b3e3ffc5776352020ff0dd380011f2d378364d3a 100644 (file)
--- a/src/backend/utils/adt/tsginidx.c
+++ b/src/backend/utils/adt/tsginidx.c
@@ -248,7 +248,7 @@ gin_tsquery_consistent(PG_FUNCTION_ARGS)
 
        res = TS_execute(GETQUERY(query),
                         &gcv,
-                        TS_EXEC_CALC_NOT | TS_EXEC_PHRASE_NO_POS,
+                        TS_EXEC_PHRASE_NO_POS,
                         checkcondition_gin);
    }
 
@@ -286,7 +286,7 @@ gin_tsquery_triconsistent(PG_FUNCTION_ARGS)
 
        if (TS_execute(GETQUERY(query),
                       &gcv,
-                      TS_EXEC_CALC_NOT | TS_EXEC_PHRASE_NO_POS,
+                      TS_EXEC_PHRASE_NO_POS,
                       checkcondition_gin))
            res = recheck ? GIN_MAYBE : GIN_TRUE;
    }

diff --git a/src/backend/utils/adt/tsgistidx.c b/src/backend/utils/adt/tsgistidx.c
index 927aed91564985033c889c9f5bc1598708de10d4..a601965bd83e6d3775e3b6e52f69e469affd492b 100644 (file)
--- a/src/backend/utils/adt/tsgistidx.c
+++ b/src/backend/utils/adt/tsgistidx.c
@@ -348,7 +348,7 @@ gtsvector_consistent(PG_FUNCTION_ARGS)
 
        PG_RETURN_BOOL(TS_execute(GETQUERY(query),
                                  key,
-                                 TS_EXEC_PHRASE_NO_POS | TS_EXEC_CALC_NOT,
+                                 TS_EXEC_PHRASE_NO_POS,
                                  checkcondition_bit));
    }
    else
@@ -359,7 +359,7 @@ gtsvector_consistent(PG_FUNCTION_ARGS)
        chkval.arre = chkval.arrb + ARRNELEM(key);
        PG_RETURN_BOOL(TS_execute(GETQUERY(query),
                                  (void *) &chkval,
-                                 TS_EXEC_PHRASE_NO_POS | TS_EXEC_CALC_NOT,
+                                 TS_EXEC_PHRASE_NO_POS,
                                  checkcondition_arr));
    }
 }

diff --git a/src/backend/utils/adt/tsrank.c b/src/backend/utils/adt/tsrank.c
index cbd97abccde4fda4320cdf3c639fd8cd4499568e..c88ebfc7d4113f31debd1de995f3e637974b78c6 100644 (file)
--- a/src/backend/utils/adt/tsrank.c
+++ b/src/backend/utils/adt/tsrank.c
@@ -697,7 +697,7 @@ Cover(DocRepresentation *doc, int len, QueryRepresentation *qr, CoverExt *ext)
        fillQueryRepresentationData(qr, ptr);
 
        if (TS_execute(GETQUERY(qr->query), (void *) qr,
-                      TS_EXEC_CALC_NOT, checkcondition_QueryOperand))
+                      TS_EXEC_EMPTY, checkcondition_QueryOperand))
        {
            if (WEP_GETPOS(ptr->pos) < ext->p)
            {

diff --git a/src/backend/utils/adt/tsvector_op.c b/src/backend/utils/adt/tsvector_op.c
index 6df943abd4e1bab3f341104f77ae583188075d39..f01b1ee25377fe39328aa05881dc447f2160a9c0 100644 (file)
--- a/src/backend/utils/adt/tsvector_op.c
+++ b/src/backend/utils/adt/tsvector_op.c
@@ -1627,9 +1627,9 @@ TS_phrase_execute(QueryItem *curitem, void *arg, uint32 flags,
             * We need not touch data->width, since a NOT operation does not
             * change the match width.
             */
-           if (!(flags & TS_EXEC_CALC_NOT))
+           if (flags & TS_EXEC_SKIP_NOT)
            {
-               /* without CALC_NOT, report NOT as "match everywhere" */
+               /* with SKIP_NOT, report NOT as "match everywhere" */
                Assert(data->npos == 0 && !data->negate);
                data->negate = true;
                return TS_YES;
@@ -1875,7 +1875,7 @@ TS_execute_recurse(QueryItem *curitem, void *arg, uint32 flags,
    switch (curitem->qoperator.oper)
    {
        case OP_NOT:
-           if (!(flags & TS_EXEC_CALC_NOT))
+           if (flags & TS_EXEC_SKIP_NOT)
                return TS_YES;
            switch (TS_execute_recurse(curitem + 1, arg, flags, chkcond))
            {
@@ -2038,7 +2038,7 @@ ts_match_vq(PG_FUNCTION_ARGS)
    chkval.operand = GETOPERAND(query);
    result = TS_execute(GETQUERY(query),
                        &chkval,
-                       TS_EXEC_CALC_NOT,
+                       TS_EXEC_EMPTY,
                        checkcondition_str);
 
    PG_FREE_IF_COPY(val, 0);

diff --git a/src/include/tsearch/ts_utils.h b/src/include/tsearch/ts_utils.h
index 609b0c7e9bdf916d45164db472c9ae3280e6afeb..400ba3300148427e7877881a021cb319eddd3608 100644 (file)
--- a/src/include/tsearch/ts_utils.h
+++ b/src/include/tsearch/ts_utils.h
@@ -183,10 +183,12 @@ typedef TSTernaryValue (*TSExecuteCallback) (void *arg, QueryOperand *val,
  */
 #define TS_EXEC_EMPTY          (0x00)
 /*
- * If TS_EXEC_CALC_NOT is not set, then NOT expressions are automatically
- * evaluated to be true.  Useful in cases where NOT isn't important (ranking).
+ * If TS_EXEC_SKIP_NOT is set, then NOT sub-expressions are automatically
+ * evaluated to be true.  This was formerly the default behavior.  It's now
+ * deprecated because it tends to give silly answers, but some applications
+ * might still have a use for it.
  */
-#define TS_EXEC_CALC_NOT       (0x01)
+#define TS_EXEC_SKIP_NOT       (0x01)
 /*
  * If TS_EXEC_PHRASE_NO_POS is set, allow OP_PHRASE to be executed lossily
  * in the absence of position information: a true result indicates that the