Re: [PATCH] fix GIN index search sometimes losing results

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: Pavel Borisov <pashkin.elfe@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2020-07-01T19:16:22Z
Lists: pgsql-hackers
Pavel Borisov <pashkin.elfe@gmail.com> writes:
> Below is my variant how to patch Gin-Gist weights issue:

I looked at this patch, but I'm unimpressed, because it's buggy.
You would have noticed if you'd included the test cases I wrote:

--- /home/postgres/pgsql/src/test/regress/expected/tsearch.out  2020-07-01 14:58
:56.637627628 -0400
+++ /home/postgres/pgsql/src/test/regress/results/tsearch.out   2020-07-01 14:59
:10.996990037 -0400
@@ -1008,13 +1008,13 @@
 SELECT count(*) FROM test_tsvector WHERE a @@ '!wd:A';
  count 
 -------
-   452
+     2
 (1 row)
 
 SELECT count(*) FROM test_tsvector WHERE a @@ '!wd:D';
  count 
 -------
-   450
+     0
 (1 row)
 
 -- Test optimization of non-empty GIN_SEARCH_MODE_ALL queries



In general, I'm not very convinced by your arguments about preserving the
option for external TS_execute callers to still use bool flags/results.
Given what we've seen so far, it seems almost certain that any such code
is buggy and needs to be rewritten anyway.  Converting to ternary logic
is far more likely to produce non-buggy code than if we continue to
try to put band-aids on the wounds.

Also, at this point I feel like it's a bit late to consider putting
anything API-breaking in v13.  But if this is a HEAD-only patch then
the argument for preserving API is even weaker.

			regards, tom lane



Commits

  1. Replace TS_execute's TS_EXEC_CALC_NOT flag with TS_EXEC_SKIP_NOT.

  2. Fix assorted bugs by changing TS_execute's callback API to ternary logic.