Pavel Borisov <pashkin.elfe@gmail.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.