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
This commit is contained in:
Tom Lane 2020-07-24 15:43:56 -04:00
parent 92fe6895d6
commit 70eca6a9a6
5 changed files with 14 additions and 12 deletions

View File

@ -248,7 +248,7 @@ gin_tsquery_consistent(PG_FUNCTION_ARGS)
res = TS_execute(GETQUERY(query), res = TS_execute(GETQUERY(query),
&gcv, &gcv,
TS_EXEC_CALC_NOT | TS_EXEC_PHRASE_NO_POS, TS_EXEC_PHRASE_NO_POS,
checkcondition_gin); checkcondition_gin);
} }
@ -286,7 +286,7 @@ gin_tsquery_triconsistent(PG_FUNCTION_ARGS)
if (TS_execute(GETQUERY(query), if (TS_execute(GETQUERY(query),
&gcv, &gcv,
TS_EXEC_CALC_NOT | TS_EXEC_PHRASE_NO_POS, TS_EXEC_PHRASE_NO_POS,
checkcondition_gin)) checkcondition_gin))
res = recheck ? GIN_MAYBE : GIN_TRUE; res = recheck ? GIN_MAYBE : GIN_TRUE;
} }

View File

@ -348,7 +348,7 @@ gtsvector_consistent(PG_FUNCTION_ARGS)
PG_RETURN_BOOL(TS_execute(GETQUERY(query), PG_RETURN_BOOL(TS_execute(GETQUERY(query),
key, key,
TS_EXEC_PHRASE_NO_POS | TS_EXEC_CALC_NOT, TS_EXEC_PHRASE_NO_POS,
checkcondition_bit)); checkcondition_bit));
} }
else else
@ -359,7 +359,7 @@ gtsvector_consistent(PG_FUNCTION_ARGS)
chkval.arre = chkval.arrb + ARRNELEM(key); chkval.arre = chkval.arrb + ARRNELEM(key);
PG_RETURN_BOOL(TS_execute(GETQUERY(query), PG_RETURN_BOOL(TS_execute(GETQUERY(query),
(void *) &chkval, (void *) &chkval,
TS_EXEC_PHRASE_NO_POS | TS_EXEC_CALC_NOT, TS_EXEC_PHRASE_NO_POS,
checkcondition_arr)); checkcondition_arr));
} }
} }

View File

@ -697,7 +697,7 @@ Cover(DocRepresentation *doc, int len, QueryRepresentation *qr, CoverExt *ext)
fillQueryRepresentationData(qr, ptr); fillQueryRepresentationData(qr, ptr);
if (TS_execute(GETQUERY(qr->query), (void *) qr, 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) if (WEP_GETPOS(ptr->pos) < ext->p)
{ {

View File

@ -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 * We need not touch data->width, since a NOT operation does not
* change the match width. * 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); Assert(data->npos == 0 && !data->negate);
data->negate = true; data->negate = true;
return TS_YES; return TS_YES;
@ -1875,7 +1875,7 @@ TS_execute_recurse(QueryItem *curitem, void *arg, uint32 flags,
switch (curitem->qoperator.oper) switch (curitem->qoperator.oper)
{ {
case OP_NOT: case OP_NOT:
if (!(flags & TS_EXEC_CALC_NOT)) if (flags & TS_EXEC_SKIP_NOT)
return TS_YES; return TS_YES;
switch (TS_execute_recurse(curitem + 1, arg, flags, chkcond)) switch (TS_execute_recurse(curitem + 1, arg, flags, chkcond))
{ {
@ -2038,7 +2038,7 @@ ts_match_vq(PG_FUNCTION_ARGS)
chkval.operand = GETOPERAND(query); chkval.operand = GETOPERAND(query);
result = TS_execute(GETQUERY(query), result = TS_execute(GETQUERY(query),
&chkval, &chkval,
TS_EXEC_CALC_NOT, TS_EXEC_EMPTY,
checkcondition_str); checkcondition_str);
PG_FREE_IF_COPY(val, 0); PG_FREE_IF_COPY(val, 0);

View File

@ -183,10 +183,12 @@ typedef TSTernaryValue (*TSExecuteCallback) (void *arg, QueryOperand *val,
*/ */
#define TS_EXEC_EMPTY (0x00) #define TS_EXEC_EMPTY (0x00)
/* /*
* If TS_EXEC_CALC_NOT is not set, then NOT expressions are automatically * If TS_EXEC_SKIP_NOT is set, then NOT sub-expressions are automatically
* evaluated to be true. Useful in cases where NOT isn't important (ranking). * 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 * 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 * in the absence of position information: a true result indicates that the