Hello,
On Thu, Jul 2, 2020 at 8:23 PM Pavel Borisov <pashkin.elfe@gmail.com> wrote:
>
> ср, 1 июл. 2020 г. в 23:16, Tom Lane <tgl@sss.pgh.pa.us>:
>>
>> 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.
>
>
> Thank you, i'd noticed and made minor corrections in the patch. Now it should work
> correctly,
>
> As for preserving the option to use legacy bool-style calls, personally I see much
> value of not changing API ad hoc to fix something. This may not harm vanilla reseases
> but can break many possible side things like RUM index etc which I think are abundant
> around there. Furthermore if we leave legacy bool callback along with newly proposed and
> recommended for further use it will cost nothing.
>
> So I've attached a corrected patch. Also I wrote some comments to the code and added
> your test as a part of apatch. Again thank you for sharing your thoughts and advice.
>
> As always I'd appreciate everyone's opinion on the bugfix.
I haven't looked at any of the patches carefully yet. But I tried both of them.
I tried Tom's patch. To compile the RUM extension I've made few
changes to use new
TS_execute(). Speaking about backward compatibility. I also think that
it is not so important
here. And RUM alreadyhas a number of "#if PG_VERSION_NUM" directives. API breaks
from time to time and it seems inevitable.
I also tried "gin-gist-weight-patch-v4.diff". And it didn't require
changes into RUM. But as
Tom said above TS_execute() is broken already. Here is the example with
"gin-gist-weight-patch-v4.diff" and RUM:
=# create extension rum;
=# create table test (a tsvector);
=# insert into test values ('wd:1A wr:2D'), ('wd:1A wr:2D');
=# create index on test using rum (a);
=# select a from test where a @@ '!wd:D';
a
----------------
'wd':1A 'wr':2
'wd':1A 'wr':2
(2 rows)
=# set enable_seqscan to off;
=# select a from test where a @@ '!wd:D';
a
---
(0 rows)
So it seems we are losing some results with RUM as well.
--
Artur
