Query refactoring #561
Query refactoring #561
Conversation
elshize
force-pushed
the
term-weights
branch
2 times, most recently
from
eda2e12 to
088a40f
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #561 +/- ##
==========================================
+ Coverage 93.21% 93.23% +0.02%
==========================================
Files 91 90 -1
Lines 4483 4452 -31
==========================================
- Hits 4179 4151 -28
+ Misses 304 301 -3 ☔ View full report in Codecov by Sentry. |
elshize
force-pushed
the
term-weights
branch
from
088a40f to
8778efe
Compare
There was a problem hiding this comment.
This looks great, nice work.
One quick thought - do we need to worry about the Thresholds tool/use here? I assume not, since thresholds are applied on a per-query basis, but I am just trying to think of any corner cases we might have missed. It might also be worth running some regression testing on this since it's quite a sensitive (but very positive) change to the inner processing logic? I'd be happy to run some tests comparing to master in the coming week or two if that would be useful.
|
Right, I don't think thresholds should be any different than queries, but I'll give it another look. Good idea about regression tests. There's a test docker image that I created for that, but haven't finished. Maybe it's a good idea to continue with it to make it easier to repeat in the future (or even automate). |
|
I need to fix the conflicts, and after that, I'll run the docker that was just merged in the other PR to evaluate and see if there's any regression. |
Weights are now stored together with term IDs and resolved at construction time according to one of the policies. In our tools, we use the default policy that removes duplicates and sets the weight to the number of occurrences of the term in a query. Other policies are, for the time being, only available programmatically via the library API. Some legacy code used to parse and process queries has been removed in favor of the text analyzer and the new query parser. Because weights are resolved when a query object is created, I also refactored creating the cursors: now the weight is simply taken from the query.
|
@JMMackenzie The regression test was successful. Are you ok merging it? |
|
Great, let's merge! |
Weights are now stored together with term IDs and resolved at construction time according to one of the policies. In our tools, we use the default policy that removes duplicates and sets the weight to the number of occurrences of the term in a query. Other policies are, for the time being, only available programmatically via the library API.
Some legacy code used to parse and process queries has been removed in favor of the text analyzer and the new query parser.
Because weights are resolved when a query object is created, I also refactored creating the cursors: now the weight is simply taken from the query.
Fixes #501