From 70eca6a9a6df679a86f30442194cc6b858b82000 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 24 Jul 2020 15:43:56 -0400 Subject: [PATCH] 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 --- src/backend/utils/adt/tsginidx.c | 4 ++-- src/backend/utils/adt/tsgistidx.c | 4 ++-- src/backend/utils/adt/tsrank.c | 2 +- src/backend/utils/adt/tsvector_op.c | 8 ++++---- src/include/tsearch/ts_utils.h | 8 +++++--- 5 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/backend/utils/adt/tsginidx.c b/src/backend/utils/adt/tsginidx.c index 3128f0a7da..b3e3ffc577 100644 --- 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 927aed9156..a601965bd8 100644 --- 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 cbd97abccd..c88ebfc7d4 100644 --- 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 6df943abd4..f01b1ee253 100644 --- 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 609b0c7e9b..400ba33001 100644 --- 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