-
Notifications
You must be signed in to change notification settings - Fork 24.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Some queries for runtime fields #58940
Changes from all commits
0cfdf33
e474582
76d8b94
818403b
3669ec4
3a390b5
59b2a18
28944a4
7d43642
89d6352
5b353ce
23af466
acf64c8
7ee42c0
216c02c
09e6feb
4e66638
6837172
f5106b2
2e05521
bfe15f8
7c18ddb
79d41c8
e3850a7
ab6834e
4e83233
810568b
3b79704
bc80a5e
93a031f
3409621
b5b02ed
e17fef8
edc571f
f90a0c7
724aaba
6645c09
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,8 +45,7 @@ public class LeafDocLookup implements Map<String, ScriptDocValues<?>> { | |
|
||
private int docId = -1; | ||
|
||
LeafDocLookup(MapperService mapperService, Function<MappedFieldType, IndexFieldData<?>> fieldDataLookup, | ||
LeafReaderContext reader) { | ||
LeafDocLookup(MapperService mapperService, Function<MappedFieldType, IndexFieldData<?>> fieldDataLookup, LeafReaderContext reader) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you revert please? Trying to trim down changes to stuff that's outside of the runtime fields plugin |
||
this.mapperService = mapperService; | ||
this.fieldDataLookup = fieldDataLookup; | ||
this.reader = reader; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,9 +51,7 @@ public void setUp() throws Exception { | |
docValues = mock(ScriptDocValues.class); | ||
IndexFieldData<?> fieldData = createFieldData(docValues); | ||
|
||
docLookup = new LeafDocLookup(mapperService, | ||
ignored -> fieldData, | ||
null); | ||
docLookup = new LeafDocLookup(mapperService, ignored -> fieldData, null); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here, let's revert? |
||
} | ||
|
||
public void testBasicLookup() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,134 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
package org.elasticsearch.xpack.runtimefields; | ||
|
||
import org.apache.lucene.index.LeafReaderContext; | ||
import org.apache.lucene.search.ConstantScoreScorer; | ||
import org.apache.lucene.search.ConstantScoreWeight; | ||
import org.apache.lucene.search.DocIdSetIterator; | ||
import org.apache.lucene.search.IndexSearcher; | ||
import org.apache.lucene.search.Query; | ||
import org.apache.lucene.search.ScoreMode; | ||
import org.apache.lucene.search.Scorer; | ||
import org.apache.lucene.search.TwoPhaseIterator; | ||
import org.apache.lucene.search.Weight; | ||
|
||
import java.io.IOException; | ||
import java.util.Objects; | ||
import java.util.function.IntConsumer; | ||
|
||
/** | ||
* Abstract base for implementing doc values and queries against values | ||
* calculated at runtime. | ||
*/ | ||
public abstract class AbstractRuntimeValues { | ||
protected int count; | ||
private boolean sort; | ||
|
||
private int docId = -1; | ||
private int maxDoc; | ||
|
||
protected final IntConsumer leafCursor(LeafReaderContext ctx) throws IOException { | ||
IntConsumer leafLoader = newLeafLoader(ctx); | ||
docId = -1; | ||
maxDoc = ctx.reader().maxDoc(); | ||
return new IntConsumer() { | ||
@Override | ||
public void accept(int targetDocId) { | ||
if (docId == targetDocId) { | ||
return; | ||
} | ||
docId = targetDocId; | ||
count = 0; | ||
leafLoader.accept(targetDocId); | ||
if (sort) { | ||
sort(); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this looks suspiciously similar to the advance method of a doc_values impl. I think I see why it is convenient to have something that accepts the doc_id and makes the values available, I guess I find it hard to follow the indirections from NewLeafLoader to IntConsumer. I think ideally we would make it possible to call script.getResult(docId) or something along those lines. That could be called from the existing ScriptBinaryDocValues implementation which also sorts values, or from the two phase iterator on the query side. Do we ever need sorted values on the query side? |
||
} | ||
}; | ||
} | ||
|
||
protected final void alwaysSortResults() { | ||
sort = true; | ||
} | ||
|
||
protected final int docId() { | ||
return docId; | ||
} | ||
|
||
protected final int maxDoc() { | ||
return maxDoc; | ||
} | ||
|
||
protected abstract IntConsumer newLeafLoader(LeafReaderContext ctx) throws IOException; | ||
|
||
protected abstract void sort(); | ||
|
||
protected abstract class AbstractRuntimeQuery extends Query { | ||
protected final String fieldName; | ||
|
||
protected AbstractRuntimeQuery(String fieldName) { | ||
this.fieldName = fieldName; | ||
} | ||
|
||
@Override | ||
public final Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException { | ||
return new ConstantScoreWeight(this, boost) { | ||
@Override | ||
public boolean isCacheable(LeafReaderContext ctx) { | ||
return false; // scripts aren't really cacheable at this point | ||
} | ||
|
||
@Override | ||
public Scorer scorer(LeafReaderContext ctx) throws IOException { | ||
IntConsumer leafCursor = leafCursor(ctx); | ||
DocIdSetIterator approximation = DocIdSetIterator.all(ctx.reader().maxDoc()); | ||
TwoPhaseIterator twoPhase = new TwoPhaseIterator(approximation) { | ||
@Override | ||
public boolean matches() throws IOException { | ||
leafCursor.accept(approximation.docID()); | ||
return AbstractRuntimeQuery.this.matches(); | ||
} | ||
|
||
@Override | ||
public float matchCost() { | ||
// TODO we don't have a good way of estimating the complexity of the script so we just go with 9000 | ||
return approximation().cost() * 9000f; | ||
} | ||
}; | ||
return new ConstantScoreScorer(this, score(), scoreMode, twoPhase); | ||
} | ||
}; | ||
} | ||
|
||
protected abstract boolean matches(); | ||
|
||
@Override | ||
public final String toString(String field) { | ||
if (fieldName.contentEquals(field)) { | ||
return bareToString(); | ||
} | ||
return fieldName + ":" + bareToString(); | ||
} | ||
|
||
protected abstract String bareToString(); | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(fieldName); | ||
} | ||
|
||
@Override | ||
public boolean equals(Object obj) { | ||
if (obj == null || getClass() != obj.getClass()) { | ||
return false; | ||
} | ||
AbstractRuntimeQuery other = (AbstractRuntimeQuery) obj; | ||
return fieldName.equals(other.fieldName); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a TODO here, to remind us that we need to figure out if this should be merged upstream or not? once the searchlookupaware hack is removed and fielddatabuilder is adapted, we will not need this change anyways?