Skip to content

Commit

Permalink
Internal: move to lucene BoostQuery
Browse files Browse the repository at this point in the history 
Latest version of lucene deprecated Query#setBoost and Query#getBoost which made queries effectively immutable. Those methods need to be replaced with `BoostQuery` that wraps any query that needs boosting.

This commit replaces usages of setBoost with BoostQuery and adds it to forbidden-apis for prod code.

Usages of `getBoost` are only partially removed, as some will have to stay for backwards compatibility.

Closes elastic#14264
  • Loading branch information
@javanna
javanna committed Nov 9, 2015
1 parent 102e254 commit 10ddd69
Show file tree
Hide file tree
Showing 44 changed files with 380 additions and 700 deletions.
3 changes: 3 additions & 0 deletions buildSrc/src/main/resources/forbidden/core-signatures.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,6 @@ java.util.concurrent.Future#cancel(boolean)
@defaultMessage Don't try reading from paths that are not configured in Environment, resolve from Environment instead
org.elasticsearch.common.io.PathUtils#get(java.lang.String, java.lang.String[])
org.elasticsearch.common.io.PathUtils#get(java.net.URI)

@defaultMessage Don't use deprecated Query#setBoost, wrap the query into a BoostQuery instead
org.apache.lucene.search.Query#setBoost(float)
67 changes: 28 additions & 39 deletions core/src/main/java/org/apache/lucene/queries/BlendedTermQuery.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,17 @@
*/
package org.apache.lucene.queries;

import org.apache.lucene.index.IndexReader;
import org.apache.lucene.index.IndexReaderContext;
import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.index.Term;
import org.apache.lucene.index.TermContext;
import org.apache.lucene.index.TermState;
import org.apache.lucene.search.BooleanClause;
import org.apache.lucene.index.*;
import org.apache.lucene.search.*;
import org.apache.lucene.search.BooleanClause.Occur;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.DisjunctionMaxQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermQuery;
import org.apache.lucene.util.ArrayUtil;
import org.apache.lucene.util.InPlaceMergeSorter;
import org.apache.lucene.util.ToStringUtils;

import java.io.IOException;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;

/**
* BlendedTermQuery can be used to unify term statistics across
Expand Down Expand Up @@ -77,6 +69,10 @@ public BlendedTermQuery(Term[] terms, float[] boosts) {

@Override
public Query rewrite(IndexReader reader) throws IOException {
Query rewritten = super.rewrite(reader);
if (rewritten != this) {
return rewritten;
}
IndexReaderContext context = reader.getContext();
TermContext[] ctx = new TermContext[terms.length];
int[] docFreqs = new int[ctx.length];
Expand All @@ -87,9 +83,7 @@ public Query rewrite(IndexReader reader) throws IOException {

final int maxDoc = reader.maxDoc();
blend(ctx, maxDoc, reader);
Query query = topLevelQuery(terms, ctx, docFreqs, maxDoc);
query.setBoost(getBoost());
return query;
return topLevelQuery(terms, ctx, docFreqs, maxDoc);
}

protected abstract Query topLevelQuery(Term[] terms, TermContext[] ctx, int[] docFreqs, int maxDoc);
Expand Down Expand Up @@ -274,20 +268,15 @@ private Term[] equalsTerms() {
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
if (!super.equals(o)) return false;

BlendedTermQuery that = (BlendedTermQuery) o;
if (!Arrays.equals(equalsTerms(), that.equalsTerms())) return false;

return true;
return Arrays.equals(equalsTerms(), that.equalsTerms());
}

@Override
public int hashCode() {
int result = super.hashCode();
result = 31 * result + Arrays.hashCode(equalsTerms());
return result;
return Objects.hash(super.hashCode(), Arrays.hashCode(equalsTerms()));
}

public static BlendedTermQuery booleanBlendedQuery(Term[] terms, final boolean disableCoord) {
Expand All @@ -298,16 +287,16 @@ public static BlendedTermQuery booleanBlendedQuery(Term[] terms, final float[] b
return new BlendedTermQuery(terms, boosts) {
@Override
protected Query topLevelQuery(Term[] terms, TermContext[] ctx, int[] docFreqs, int maxDoc) {
BooleanQuery.Builder query = new BooleanQuery.Builder();
query.setDisableCoord(disableCoord);
BooleanQuery.Builder booleanQueryBuilder = new BooleanQuery.Builder();
booleanQueryBuilder.setDisableCoord(disableCoord);
for (int i = 0; i < terms.length; i++) {
TermQuery termQuery = new TermQuery(terms[i], ctx[i]);
if (boosts != null) {
termQuery.setBoost(boosts[i]);
Query query = new TermQuery(terms[i], ctx[i]);
if (boosts != null && boosts[i] != 1f) {
query = new BoostQuery(query, boosts[i]);
}
query.add(termQuery, BooleanClause.Occur.SHOULD);
booleanQueryBuilder.add(query, BooleanClause.Occur.SHOULD);
}
return query.build();
return booleanQueryBuilder.build();
}
};
}
Expand All @@ -321,16 +310,16 @@ protected Query topLevelQuery(Term[] terms, TermContext[] ctx, int[] docFreqs, i
BooleanQuery.Builder lowBuilder = new BooleanQuery.Builder();
lowBuilder.setDisableCoord(disableCoord);
for (int i = 0; i < terms.length; i++) {
TermQuery termQuery = new TermQuery(terms[i], ctx[i]);
if (boosts != null) {
termQuery.setBoost(boosts[i]);
Query query = new TermQuery(terms[i], ctx[i]);
if (boosts != null && boosts[i] != 1f) {
query = new BoostQuery(query, boosts[i]);
}
if ((maxTermFrequency >= 1f && docFreqs[i] > maxTermFrequency)
|| (docFreqs[i] > (int) Math.ceil(maxTermFrequency
* (float) maxDoc))) {
highBuilder.add(termQuery, BooleanClause.Occur.SHOULD);
highBuilder.add(query, BooleanClause.Occur.SHOULD);
} else {
lowBuilder.add(termQuery, BooleanClause.Occur.SHOULD);
lowBuilder.add(query, BooleanClause.Occur.SHOULD);
}
}
BooleanQuery high = highBuilder.build();
Expand Down Expand Up @@ -363,15 +352,15 @@ public static BlendedTermQuery dismaxBlendedQuery(Term[] terms, final float[] bo
return new BlendedTermQuery(terms, boosts) {
@Override
protected Query topLevelQuery(Term[] terms, TermContext[] ctx, int[] docFreqs, int maxDoc) {
DisjunctionMaxQuery query = new DisjunctionMaxQuery(tieBreakerMultiplier);
DisjunctionMaxQuery disMaxQuery = new DisjunctionMaxQuery(tieBreakerMultiplier);
for (int i = 0; i < terms.length; i++) {
TermQuery termQuery = new TermQuery(terms[i], ctx[i]);
if (boosts != null) {
termQuery.setBoost(boosts[i]);
Query query = new TermQuery(terms[i], ctx[i]);
if (boosts != null && boosts[i] != 1f) {
query = new BoostQuery(query, boosts[i]);
}
query.add(termQuery);
disMaxQuery.add(query);
}
return query;
return disMaxQuery;
}
};
}
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/java/org/apache/lucene/queries/MinDocQuery.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.apache.lucene.search.Weight;

import java.io.IOException;
import java.util.Objects;

/** A {@link Query} that only matches documents that are greater than or equal
* to a configured doc ID. */
Expand All @@ -43,7 +44,7 @@ public MinDocQuery(int minDoc) {

@Override
public int hashCode() {
return 31 * super.hashCode() + minDoc;
return Objects.hash(super.hashCode(), minDoc);
}

@Override
Expand Down
70 changes: 25 additions & 45 deletions core/src/main/java/org/apache/lucene/queryparser/classic/MapperQueryParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,7 @@
import org.apache.lucene.analysis.TokenStream;
import org.apache.lucene.analysis.tokenattributes.CharTermAttribute;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.BooleanClause;
import org.apache.lucene.search.DisjunctionMaxQuery;
import org.apache.lucene.search.FuzzyQuery;
import org.apache.lucene.search.MatchNoDocsQuery;
import org.apache.lucene.search.MultiPhraseQuery;
import org.apache.lucene.search.PhraseQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.*;
import org.apache.lucene.util.IOUtils;
import org.apache.lucene.util.automaton.RegExp;
import org.elasticsearch.common.lucene.search.Queries;
Expand All @@ -41,12 +35,7 @@
import org.elasticsearch.index.query.support.QueryParsers;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.*;

import static java.util.Collections.unmodifiableMap;
import static org.elasticsearch.common.lucene.search.Queries.fixNegativeQueryIfNeeded;
Expand Down Expand Up @@ -148,8 +137,7 @@ public Query getFieldQuery(String field, String queryText, boolean quoted) throw
Query q = getFieldQuerySingle(mField, queryText, quoted);
if (q != null) {
added = true;
applyBoost(mField, q);
disMaxQuery.add(q);
disMaxQuery.add(applyBoost(mField, q));
}
}
if (!added) {
Expand All @@ -161,8 +149,7 @@ public Query getFieldQuery(String field, String queryText, boolean quoted) throw
for (String mField : fields) {
Query q = getFieldQuerySingle(mField, queryText, quoted);
if (q != null) {
applyBoost(mField, q);
clauses.add(new BooleanClause(q, BooleanClause.Occur.SHOULD));
clauses.add(new BooleanClause(applyBoost(mField, q), BooleanClause.Occur.SHOULD));
}
}
if (clauses.size() == 0) // happens for stopwords
Expand Down Expand Up @@ -250,9 +237,8 @@ protected Query getFieldQuery(String field, String queryText, int slop) throws P
Query q = super.getFieldQuery(mField, queryText, slop);
if (q != null) {
added = true;
applyBoost(mField, q);
q = applySlop(q, slop);
disMaxQuery.add(q);
disMaxQuery.add(applyBoost(mField, q));
}
}
if (!added) {
Expand All @@ -264,9 +250,8 @@ protected Query getFieldQuery(String field, String queryText, int slop) throws P
for (String mField : fields) {
Query q = super.getFieldQuery(mField, queryText, slop);
if (q != null) {
applyBoost(mField, q);
q = applySlop(q, slop);
clauses.add(new BooleanClause(q, BooleanClause.Occur.SHOULD));
clauses.add(new BooleanClause(applyBoost(mField, q), BooleanClause.Occur.SHOULD));
}
}
if (clauses.size() == 0) // happens for stopwords
Expand Down Expand Up @@ -305,8 +290,7 @@ protected Query getRangeQuery(String field, String part1, String part2, boolean
Query q = getRangeQuerySingle(mField, part1, part2, startInclusive, endInclusive);
if (q != null) {
added = true;
applyBoost(mField, q);
disMaxQuery.add(q);
disMaxQuery.add(applyBoost(mField, q));
}
}
if (!added) {
Expand All @@ -318,8 +302,7 @@ protected Query getRangeQuery(String field, String part1, String part2, boolean
for (String mField : fields) {
Query q = getRangeQuerySingle(mField, part1, part2, startInclusive, endInclusive);
if (q != null) {
applyBoost(mField, q);
clauses.add(new BooleanClause(q, BooleanClause.Occur.SHOULD));
clauses.add(new BooleanClause(applyBoost(mField, q), BooleanClause.Occur.SHOULD));
}
}
if (clauses.size() == 0) // happens for stopwords
Expand Down Expand Up @@ -371,8 +354,7 @@ protected Query getFuzzyQuery(String field, String termStr, String minSimilarity
Query q = getFuzzyQuerySingle(mField, termStr, minSimilarity);
if (q != null) {
added = true;
applyBoost(mField, q);
disMaxQuery.add(q);
disMaxQuery.add(applyBoost(mField, q));
}
}
if (!added) {
Expand All @@ -383,8 +365,9 @@ protected Query getFuzzyQuery(String field, String termStr, String minSimilarity
List<BooleanClause> clauses = new ArrayList<>();
for (String mField : fields) {
Query q = getFuzzyQuerySingle(mField, termStr, minSimilarity);
applyBoost(mField, q);
clauses.add(new BooleanClause(q, BooleanClause.Occur.SHOULD));
if (q != null) {
clauses.add(new BooleanClause(applyBoost(mField, q), BooleanClause.Occur.SHOULD));
}
}
return getBooleanQuery(clauses, true);
}
Expand Down Expand Up @@ -434,8 +417,7 @@ protected Query getPrefixQuery(String field, String termStr) throws ParseExcepti
Query q = getPrefixQuerySingle(mField, termStr);
if (q != null) {
added = true;
applyBoost(mField, q);
disMaxQuery.add(q);
disMaxQuery.add(applyBoost(mField, q));
}
}
if (!added) {
Expand All @@ -447,8 +429,7 @@ protected Query getPrefixQuery(String field, String termStr) throws ParseExcepti
for (String mField : fields) {
Query q = getPrefixQuerySingle(mField, termStr);
if (q != null) {
applyBoost(mField, q);
clauses.add(new BooleanClause(q, BooleanClause.Occur.SHOULD));
clauses.add(new BooleanClause(applyBoost(mField, q), BooleanClause.Occur.SHOULD));
}
}
if (clauses.size() == 0) // happens for stopwords
Expand Down Expand Up @@ -566,8 +547,7 @@ protected Query getWildcardQuery(String field, String termStr) throws ParseExcep
Query q = getWildcardQuerySingle(mField, termStr);
if (q != null) {
added = true;
applyBoost(mField, q);
disMaxQuery.add(q);
disMaxQuery.add(applyBoost(mField, q));
}
}
if (!added) {
Expand All @@ -579,8 +559,7 @@ protected Query getWildcardQuery(String field, String termStr) throws ParseExcep
for (String mField : fields) {
Query q = getWildcardQuerySingle(mField, termStr);
if (q != null) {
applyBoost(mField, q);
clauses.add(new BooleanClause(q, BooleanClause.Occur.SHOULD));
clauses.add(new BooleanClause(applyBoost(mField, q), BooleanClause.Occur.SHOULD));
}
}
if (clauses.size() == 0) // happens for stopwords
Expand Down Expand Up @@ -697,8 +676,7 @@ protected Query getRegexpQuery(String field, String termStr) throws ParseExcepti
Query q = getRegexpQuerySingle(mField, termStr);
if (q != null) {
added = true;
applyBoost(mField, q);
disMaxQuery.add(q);
disMaxQuery.add(applyBoost(mField, q));
}
}
if (!added) {
Expand All @@ -710,8 +688,7 @@ protected Query getRegexpQuery(String field, String termStr) throws ParseExcepti
for (String mField : fields) {
Query q = getRegexpQuerySingle(mField, termStr);
if (q != null) {
applyBoost(mField, q);
clauses.add(new BooleanClause(q, BooleanClause.Occur.SHOULD));
clauses.add(new BooleanClause(applyBoost(mField, q), BooleanClause.Occur.SHOULD));
}
}
if (clauses.size() == 0) // happens for stopwords
Expand Down Expand Up @@ -761,11 +738,12 @@ protected Query getBooleanQuery(List<BooleanClause> clauses, boolean disableCoor
return fixNegativeQueryIfNeeded(q);
}

private void applyBoost(String field, Query q) {
private Query applyBoost(String field, Query q) {
Float fieldBoost = settings.fieldsAndWeights().get(field);
if (fieldBoost != null) {
q.setBoost(fieldBoost);
if (fieldBoost != null && fieldBoost != 1f) {
return new BoostQuery(q, fieldBoost);
}
return q;
}

private Query applySlop(Query q, int slop) {
Expand All @@ -779,7 +757,9 @@ private Query applySlop(Query q, int slop) {
builder.add(terms[i], positions[i]);
}
pq = builder.build();
pq.setBoost(q.getBoost());
//make sure that the boost hasn't been set beforehand, otherwise we'd lose it
assert q.getBoost() == 1f;
assert q instanceof BoostQuery == false;
return pq;
} else if (q instanceof MultiPhraseQuery) {
((MultiPhraseQuery) q).setSlop(slop);
Expand Down
9 changes: 2 additions & 7 deletions core/src/main/java/org/apache/lucene/search/vectorhighlight/CustomFieldQuery.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,7 @@
import org.apache.lucene.index.IndexReader;
import org.apache.lucene.index.Term;
import org.apache.lucene.queries.BlendedTermQuery;
import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.search.MultiPhraseQuery;
import org.apache.lucene.search.PhraseQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermQuery;
import org.apache.lucene.search.*;
import org.apache.lucene.search.spans.SpanTermQuery;
import org.elasticsearch.common.lucene.search.MultiPhrasePrefixQuery;
import org.elasticsearch.common.lucene.search.function.FiltersFunctionScoreQuery;
Expand Down Expand Up @@ -103,8 +99,7 @@ private void convertMultiPhraseQuery(int currentPos, int[] termsIdx, MultiPhrase
for (int i = 0; i < termsIdx.length; i++) {
queryBuilder.add(terms.get(i)[termsIdx[i]], pos[i]);
}
PhraseQuery query = queryBuilder.build();
query.setBoost(orig.getBoost());
Query query = queryBuilder.build();
this.flatten(query, reader, flatQueries, orig.getBoost());
} else {
Term[] t = terms.get(currentPos);
Expand Down
Loading

0 comments on commit 10ddd69

Please sign in to comment.