Skip to content

Commit ae7cd5e

Browse files
javannajavanna
authored
Replace MappedFieldType#extractTerm with local query visitor (#113163)
The only usage of `MappedFieldType#extractTerm` comes from `SpanTermQueryBuilder` which attempts to extract a single term from a generic Query obtained from calling `MappedFieldType#termQuery`. We can move this logic directly within its only caller, and instead of using instanceof checks, we can rely on the query visitor API. This additionally allows us to remove one of the leftover usages of TermInSetQuery#getTermData which is deprecated in Lucene
1 parent f4f075a commit ae7cd5e

File tree

4 files changed

+62
-45
lines changed

4 files changed

+62
-45
lines changed

Diff for: ‎qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/upgrades/QueryBuilderBWCIT.java

+12-17
Original file line numberDiff line numberDiff line change
@@ -154,29 +154,29 @@ public QueryBuilderBWCIT(@Name("cluster") FullClusterRestartUpgradeStatus upgrad
154154
);
155155
addCandidate(
156156
"""
157-
"span_near": {"clauses": [{ "span_term": { "keyword_field": "value1" }}, \
158-
{ "span_term": { "keyword_field": "value2" }}]}
157+
"span_near": {"clauses": [{ "span_term": { "text_field": "value1" }}, \
158+
{ "span_term": { "text_field": "value2" }}]}
159159
""",
160-
new SpanNearQueryBuilder(new SpanTermQueryBuilder("keyword_field", "value1"), 0).addClause(
161-
new SpanTermQueryBuilder("keyword_field", "value2")
160+
new SpanNearQueryBuilder(new SpanTermQueryBuilder("text_field", "value1"), 0).addClause(
161+
new SpanTermQueryBuilder("text_field", "value2")
162162
)
163163
);
164164
addCandidate(
165165
"""
166-
"span_near": {"clauses": [{ "span_term": { "keyword_field": "value1" }}, \
167-
{ "span_term": { "keyword_field": "value2" }}], "slop": 2}
166+
"span_near": {"clauses": [{ "span_term": { "text_field": "value1" }}, \
167+
{ "span_term": { "text_field": "value2" }}], "slop": 2}
168168
""",
169-
new SpanNearQueryBuilder(new SpanTermQueryBuilder("keyword_field", "value1"), 2).addClause(
170-
new SpanTermQueryBuilder("keyword_field", "value2")
169+
new SpanNearQueryBuilder(new SpanTermQueryBuilder("text_field", "value1"), 2).addClause(
170+
new SpanTermQueryBuilder("text_field", "value2")
171171
)
172172
);
173173
addCandidate(
174174
"""
175-
"span_near": {"clauses": [{ "span_term": { "keyword_field": "value1" }}, \
176-
{ "span_term": { "keyword_field": "value2" }}], "slop": 2, "in_order": false}
175+
"span_near": {"clauses": [{ "span_term": { "text_field": "value1" }}, \
176+
{ "span_term": { "text_field": "value2" }}], "slop": 2, "in_order": false}
177177
""",
178-
new SpanNearQueryBuilder(new SpanTermQueryBuilder("keyword_field", "value1"), 2).addClause(
179-
new SpanTermQueryBuilder("keyword_field", "value2")
178+
new SpanNearQueryBuilder(new SpanTermQueryBuilder("text_field", "value1"), 2).addClause(
179+
new SpanTermQueryBuilder("text_field", "value2")
180180
).inOrder(false)
181181
);
182182
}
@@ -204,11 +204,6 @@ public void testQueryBuilderBWC() throws Exception {
204204
mappingsAndSettings.field("type", "percolator");
205205
mappingsAndSettings.endObject();
206206
}
207-
{
208-
mappingsAndSettings.startObject("keyword_field");
209-
mappingsAndSettings.field("type", "keyword");
210-
mappingsAndSettings.endObject();
211-
}
212207
{
213208
mappingsAndSettings.startObject("text_field");
214209
mappingsAndSettings.field("type", "text");

Diff for: ‎server/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java

-27
Original file line numberDiff line numberDiff line change
@@ -12,21 +12,17 @@
1212
import org.apache.lucene.analysis.TokenStream;
1313
import org.apache.lucene.index.FieldInfos;
1414
import org.apache.lucene.index.IndexReader;
15-
import org.apache.lucene.index.PrefixCodedTerms;
16-
import org.apache.lucene.index.PrefixCodedTerms.TermIterator;
1715
import org.apache.lucene.index.Term;
1816
import org.apache.lucene.index.TermsEnum;
1917
import org.apache.lucene.queries.intervals.IntervalsSource;
2018
import org.apache.lucene.queries.spans.SpanMultiTermQueryWrapper;
2119
import org.apache.lucene.queries.spans.SpanQuery;
2220
import org.apache.lucene.search.BooleanClause.Occur;
2321
import org.apache.lucene.search.BooleanQuery;
24-
import org.apache.lucene.search.BoostQuery;
2522
import org.apache.lucene.search.ConstantScoreQuery;
2623
import org.apache.lucene.search.FieldExistsQuery;
2724
import org.apache.lucene.search.MultiTermQuery;
2825
import org.apache.lucene.search.Query;
29-
import org.apache.lucene.search.TermInSetQuery;
3026
import org.apache.lucene.search.TermQuery;
3127
import org.apache.lucene.util.BytesRef;
3228
import org.elasticsearch.ElasticsearchException;
@@ -561,29 +557,6 @@ protected void checkNoTimeZone(@Nullable ZoneId timeZone) {
561557
}
562558
}
563559

564-
/**
565-
* Extract a {@link Term} from a query created with {@link #termQuery} by
566-
* recursively removing {@link BoostQuery} wrappers.
567-
* @throws IllegalArgumentException if the wrapped query is not a {@link TermQuery}
568-
*/
569-
public static Term extractTerm(Query termQuery) {
570-
while (termQuery instanceof BoostQuery) {
571-
termQuery = ((BoostQuery) termQuery).getQuery();
572-
}
573-
if (termQuery instanceof TermInSetQuery tisQuery) {
574-
PrefixCodedTerms terms = tisQuery.getTermData();
575-
if (terms.size() == 1) {
576-
TermIterator it = terms.iterator();
577-
BytesRef term = it.next();
578-
return new Term(it.field(), term);
579-
}
580-
}
581-
if (termQuery instanceof TermQuery == false) {
582-
throw new IllegalArgumentException("Cannot extract a term from a query of type " + termQuery.getClass() + ": " + termQuery);
583-
}
584-
return ((TermQuery) termQuery).getTerm();
585-
}
586-
587560
/**
588561
* Get the metadata associated with this field.
589562
*/

Diff for: ‎server/src/main/java/org/elasticsearch/index/query/SpanTermQueryBuilder.java

+30-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212
import org.apache.lucene.index.Term;
1313
import org.apache.lucene.queries.spans.SpanQuery;
1414
import org.apache.lucene.queries.spans.SpanTermQuery;
15+
import org.apache.lucene.search.BooleanClause;
1516
import org.apache.lucene.search.Query;
17+
import org.apache.lucene.search.QueryVisitor;
1618
import org.elasticsearch.TransportVersion;
1719
import org.elasticsearch.TransportVersions;
1820
import org.elasticsearch.common.ParsingException;
@@ -23,6 +25,9 @@
2325
import org.elasticsearch.xcontent.XContentParser;
2426

2527
import java.io.IOException;
28+
import java.util.ArrayList;
29+
import java.util.Arrays;
30+
import java.util.List;
2631

2732
/**
2833
* A Span Query that matches documents containing a term.
@@ -77,8 +82,32 @@ protected SpanQuery doToQuery(SearchExecutionContext context) throws IOException
7782
if (mapper == null) {
7883
term = new Term(fieldName, BytesRefs.toBytesRef(value));
7984
} else {
85+
if (mapper.getTextSearchInfo().hasPositions() == false) {
86+
throw new IllegalArgumentException(
87+
"Span term query requires position data, but field " + fieldName + " was indexed without position data"
88+
);
89+
}
8090
Query termQuery = mapper.termQuery(value, context);
81-
term = MappedFieldType.extractTerm(termQuery);
91+
List<Term> termsList = new ArrayList<>();
92+
termQuery.visit(new QueryVisitor() {
93+
@Override
94+
public QueryVisitor getSubVisitor(BooleanClause.Occur occur, Query parent) {
95+
if (occur == BooleanClause.Occur.MUST || occur == BooleanClause.Occur.FILTER) {
96+
return this;
97+
}
98+
return EMPTY_VISITOR;
99+
}
100+
101+
@Override
102+
public void consumeTerms(Query query, Term... terms) {
103+
termsList.addAll(Arrays.asList(terms));
104+
}
105+
});
106+
if (termsList.size() != 1) {
107+
// This is for safety, but we have called mapper.termQuery above: we really should get one and only one term from the query?
108+
throw new IllegalArgumentException("Cannot extract a term from a query of type " + termQuery.getClass() + ": " + termQuery);
109+
}
110+
term = termsList.get(0);
82111
}
83112
return new SpanTermQuery(term);
84113
}

Diff for: ‎server/src/test/java/org/elasticsearch/index/query/SpanTermQueryBuilderTests.java

+20
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,12 @@
1111

1212
import org.apache.lucene.index.Term;
1313
import org.apache.lucene.queries.spans.SpanTermQuery;
14+
import org.apache.lucene.search.BoostQuery;
1415
import org.apache.lucene.search.Query;
1516
import org.apache.lucene.search.TermQuery;
1617
import org.elasticsearch.common.ParsingException;
1718
import org.elasticsearch.common.lucene.BytesRefs;
19+
import org.elasticsearch.index.mapper.IdFieldMapper;
1820
import org.elasticsearch.index.mapper.MappedFieldType;
1921
import org.elasticsearch.xcontent.json.JsonStringEncoder;
2022

@@ -124,4 +126,22 @@ public void testWithMetadataField() throws IOException {
124126
assertEquals(expected, query);
125127
}
126128
}
129+
130+
public void testWithBoost() throws IOException {
131+
SearchExecutionContext context = createSearchExecutionContext();
132+
for (String field : new String[] { "field1", "field2" }) {
133+
SpanTermQueryBuilder spanTermQueryBuilder = new SpanTermQueryBuilder(field, "toto");
134+
spanTermQueryBuilder.boost(10);
135+
Query query = spanTermQueryBuilder.toQuery(context);
136+
Query expected = new BoostQuery(new SpanTermQuery(new Term(field, "toto")), 10);
137+
assertEquals(expected, query);
138+
}
139+
}
140+
141+
public void testFieldWithoutPositions() {
142+
SearchExecutionContext context = createSearchExecutionContext();
143+
SpanTermQueryBuilder spanTermQueryBuilder = new SpanTermQueryBuilder(IdFieldMapper.NAME, "1234");
144+
IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> spanTermQueryBuilder.toQuery(context));
145+
assertEquals("Span term query requires position data, but field _id was indexed without position data", iae.getMessage());
146+
}
127147
}

0 commit comments

Comments
 (0)