Implement distance_feature for runtime dates#60851
Implement distance_feature for runtime dates#60851nik9000 merged 5 commits intoelastic:feature/runtime_fieldsfrom
Conversation
|
Pinging @elastic/es-search (:Search/Search) |
mayya-sharipova
left a comment
There was a problem hiding this comment.
Thanks @nik9000, I have left a couple of comments
|
|
||
| @Override | ||
| public float getMaxScore(int upTo) throws IOException { | ||
| return boost; |
There was a problem hiding this comment.
Should a potential max score be: weight instead of boost?
This method and several other methods don't seem to throw IOException
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(super.hashCode(), origin, pivot); |
There was a problem hiding this comment.
should we incorporate the initial boost to hashCode, equals, toString ?
| protected DistanceScorer(Weight weight, AbstractLongScriptFieldScript script, int maxDoc, float boost) { | ||
| super(weight); | ||
| this.script = script; | ||
| twoPhase = new TwoPhaseIterator(DocIdSetIterator.all(maxDoc)) { |
There was a problem hiding this comment.
I am not familiar with runtime fields, but I am wondering if we intend always to create an iterator across all documents? Do we plan to add support to limit number of docs (e.g. only docs returned by a top filter)?
There was a problem hiding this comment.
I believe we're hoping for bool queries to AND together a "normal" query and a runtime field query. I've experimented with this for our term and match style queries and it seems to work pretty well. If the "normal" query is selective then the runtime query won't be asked if it matches most documents. On the flip side, if the runtime field query non-selective then we'll quickly fill up the 10,000 hits and terminate early.
nik9000
left a comment
There was a problem hiding this comment.
I'll push a patch to address your comments soon!
|
|
||
| @Override | ||
| public float getMaxScore(int upTo) throws IOException { | ||
| return boost; |
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(super.hashCode(), origin, pivot); |
| protected DistanceScorer(Weight weight, AbstractLongScriptFieldScript script, int maxDoc, float boost) { | ||
| super(weight); | ||
| this.script = script; | ||
| twoPhase = new TwoPhaseIterator(DocIdSetIterator.all(maxDoc)) { |
There was a problem hiding this comment.
I believe we're hoping for bool queries to AND together a "normal" query and a runtime field query. I've experimented with this for our term and match style queries and it seems to work pretty well. If the "normal" query is selective then the runtime query won't be asked if it matches most documents. On the flip side, if the runtime field query non-selective then we'll quickly fill up the 10,000 hits and terminate early.
| boost | ||
| ); | ||
| }); | ||
| } |
There was a problem hiding this comment.
I wonder what the plan is for the instanceof checks in DistanceFeatureQueryBuilder#doToQuery . Are we ok with keeping those?
There was a problem hiding this comment.
oh actually those are gone upstream, great! sorry for the noise then, you already did what I would asked you to do
This implements the distance_feature for
datevaluedruntime_scripts. This produces the same numbers running against an indexed date, but it doesn't have the same performance characteristics at all. Which is normal forruntime_scripts. Butdistance_feature` against an indexes fields does a lot of work to refine the query as it goes, limiting the number of documents that it has to visit. We can't do that because we don't have an index. So we just spit out the same numbers and hope it is good enough.