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

ср, 22 июл. 2020 г. в 19:10, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:

> Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com> writes:
> > For 0002-remove-calc-not-flag.patch
> > The patch changes the behavior which is now considered default. This is
> true in RUM module and maybe in some other tsearch side modules. Applying
> the patch can make code more beautiful but possibly will not give some
> performance gain and bug is anyway fixed by patch 0001.
>
> I'd be willing to compromise on just adding TS_EXEC_CALC_NOT to the
> calls that are missing it today. But I don't see why that's really
> a great idea --- it still leaves a risk-of-omission hazard for future
> callers. Calculating NOTs correctly really ought to be the default
> behavior.
>
> What do you think of replacing TS_EXEC_CALC_NOT with a different
> flag having the opposite sense, maybe called TS_EXEC_SKIP_NOT?
> If anyone really does need that behavior, they could still get it,
> but they'd have to be explicit.
>
> > Overall I'd recommend patch 0001-make-callbacks-ternary.patch and close
> the issue.
>
> The other issue we have to agree on is whether we want to sneak this
> fix into v13, or wait another year for it. I feel like it's pretty
> late to be making potentially API-breaking changes, but on the other
> hand this is undoubtedly a bug fix.
>
> regards, tom lane
>

I am convinced patch 0001 is necessary and enough to fix a bug, so I think
it's very much worth adding it to v13.

As for 0002 I see the beauty of this change but I also see the value of
leaving defaults as they were before.
The change of CALC_NOT behavior doesn't seem to be a source of big changes,
though. I'm just not convinced it is very much needed.
The best way I think is to leave 0002 until the next version and add
commentary in the code that this default behavior of NOT
doing TS_EXEC_CALC_NOT is inherited from past, so basically any caller
should set this flag (see patch 0003-add-comments-on-calc-not.

Best regards,
Pavel Borisov

Postgres Professional: http://postgrespro.com