Skip to content

Commit 9aae62b

Browse files
committed
All Field: Automatically detect when field level boosting is used, and optimize when its not, closes elastic#2189.
1 parent e1fe893 commit 9aae62b

File tree

6 files changed

+111
-9
lines changed

6 files changed

+111
-9
lines changed

src/main/java/org/elasticsearch/common/lucene/all/AllEntries.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ public void addText(String name, String text, float boost) {
7979
entries.add(entry);
8080
}
8181

82+
public boolean customBoost() {
83+
return customBoost;
84+
}
85+
8286
public void clear() {
8387
this.entries.clear();
8488
this.current = null;

src/main/java/org/elasticsearch/index/mapper/internal/AllFieldMapper.java

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,9 @@ public static class Builder extends AbstractFieldMapper.Builder<Builder, AllFiel
6969

7070
private boolean enabled = Defaults.ENABLED;
7171

72+
// an internal flag, automatically set if we encounter boosting
73+
boolean autoBoost = false;
74+
7275
public Builder() {
7376
super(Defaults.NAME);
7477
builder = this;
@@ -103,7 +106,7 @@ protected Builder searchAnalyzer(NamedAnalyzer searchAnalyzer) {
103106
@Override
104107
public AllFieldMapper build(BuilderContext context) {
105108
return new AllFieldMapper(name, store, termVector, omitNorms, indexOptions,
106-
indexAnalyzer, searchAnalyzer, enabled);
109+
indexAnalyzer, searchAnalyzer, enabled, autoBoost);
107110
}
108111
}
109112

@@ -117,6 +120,8 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext
117120
Object fieldNode = entry.getValue();
118121
if (fieldName.equals("enabled")) {
119122
builder.enabled(nodeBooleanValue(fieldNode));
123+
} else if (fieldName.equals("auto_boost")) {
124+
builder.autoBoost = nodeBooleanValue(fieldNode);
120125
}
121126
}
122127
return builder;
@@ -125,16 +130,24 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext
125130

126131

127132
private boolean enabled;
133+
// The autoBoost flag is automatically set based on indexed docs on the mappings
134+
// if a doc is indexed with a specific boost value and part of _all, it is automatically
135+
// set to true. This allows to optimize (automatically, which we like) for the common case
136+
// where fields don't usually have boost associated with them, and we don't need to use the
137+
// special SpanTermQuery to look at payloads
138+
private volatile boolean autoBoost;
128139

129140
public AllFieldMapper() {
130-
this(Defaults.NAME, Defaults.STORE, Defaults.TERM_VECTOR, Defaults.OMIT_NORMS, Defaults.INDEX_OPTIONS, null, null, Defaults.ENABLED);
141+
this(Defaults.NAME, Defaults.STORE, Defaults.TERM_VECTOR, Defaults.OMIT_NORMS, Defaults.INDEX_OPTIONS, null, null, Defaults.ENABLED, false);
131142
}
132143

133144
protected AllFieldMapper(String name, Field.Store store, Field.TermVector termVector, boolean omitNorms, IndexOptions indexOptions,
134-
NamedAnalyzer indexAnalyzer, NamedAnalyzer searchAnalyzer, boolean enabled) {
145+
NamedAnalyzer indexAnalyzer, NamedAnalyzer searchAnalyzer, boolean enabled, boolean autoBoost) {
135146
super(new Names(name, name, name, name), Field.Index.ANALYZED, store, termVector, 1.0f, omitNorms, indexOptions, indexAnalyzer,
136147
searchAnalyzer);
137148
this.enabled = enabled;
149+
this.autoBoost = autoBoost;
150+
138151
}
139152

140153
public boolean enabled() {
@@ -143,16 +156,19 @@ public boolean enabled() {
143156

144157
@Override
145158
public Query queryStringTermQuery(Term term) {
159+
if (!autoBoost) {
160+
return new TermQuery(term);
161+
}
146162
if (indexOptions == IndexOptions.DOCS_AND_FREQS_AND_POSITIONS) {
147163
return new AllTermQuery(term);
148-
}
164+
}
149165
return new TermQuery(term);
150166
}
151167

152168
@Override
153169
public Query fieldQuery(String value, QueryParseContext context) {
154170
return queryStringTermQuery(names().createIndexNameTerm(value));
155-
171+
156172
}
157173

158174
@Override
@@ -186,6 +202,13 @@ protected Fieldable parseCreateField(ParseContext context) throws IOException {
186202
// reset the entries
187203
context.allEntries().reset();
188204

205+
// if the autoBoost flag is not set, and we indexed a doc with custom boost, make
206+
// sure to update the flag, and notify mappings on change
207+
if (!autoBoost && context.allEntries().customBoost()) {
208+
autoBoost = true;
209+
context.setMappingsModified();
210+
}
211+
189212
Analyzer analyzer = findAnalyzer(context);
190213
return new AllField(names.indexName(), store, termVector, context.allEntries(), analyzer);
191214
}
@@ -240,6 +263,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
240263
if (enabled != Defaults.ENABLED) {
241264
builder.field("enabled", enabled);
242265
}
266+
if (autoBoost != false) {
267+
builder.field("auto_boost", autoBoost);
268+
}
243269
if (store != Defaults.STORE) {
244270
builder.field("store", store.name().toLowerCase());
245271
}

src/test/java/org/elasticsearch/test/unit/index/mapper/all/SimpleAllMapperTests.java

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import org.elasticsearch.common.lucene.all.AllTokenStream;
3030
import org.elasticsearch.index.mapper.DocumentMapper;
3131
import org.elasticsearch.index.mapper.FieldMapper;
32-
import org.elasticsearch.index.mapper.FieldMappers;
3332
import org.elasticsearch.test.unit.index.mapper.MapperTests;
3433
import org.hamcrest.Matchers;
3534
import org.testng.annotations.Test;
@@ -60,7 +59,23 @@ public void testSimpleAllMappers() throws Exception {
6059
FieldMapper mapper = docMapper.mappers().smartNameFieldMapper("_all");
6160
assertThat(mapper.queryStringTermQuery(new Term("_all", "foobar")), Matchers.instanceOf(AllTermQuery.class));
6261
}
63-
62+
63+
@Test
64+
public void testAllMappersNoBoost() throws Exception {
65+
String mapping = copyToStringFromClasspath("/org/elasticsearch/test/unit/index/mapper/all/noboost-mapping.json");
66+
DocumentMapper docMapper = MapperTests.newParser().parse(mapping);
67+
byte[] json = copyToBytesFromClasspath("/org/elasticsearch/test/unit/index/mapper/all/test1.json");
68+
Document doc = docMapper.parse(new BytesArray(json)).rootDoc();
69+
AllField field = (AllField) doc.getFieldable("_all");
70+
AllEntries allEntries = ((AllTokenStream) field.tokenStreamValue()).allEntries();
71+
assertThat(allEntries.fields().size(), equalTo(3));
72+
assertThat(allEntries.fields().contains("address.last.location"), equalTo(true));
73+
assertThat(allEntries.fields().contains("name.last"), equalTo(true));
74+
assertThat(allEntries.fields().contains("simple1"), equalTo(true));
75+
FieldMapper mapper = docMapper.mappers().smartNameFieldMapper("_all");
76+
assertThat(mapper.queryStringTermQuery(new Term("_all", "foobar")), Matchers.instanceOf(TermQuery.class));
77+
}
78+
6479
@Test
6580
public void testAllMappersTermQuery() throws Exception {
6681
String mapping = copyToStringFromClasspath("/org/elasticsearch/test/unit/index/mapper/all/mapping_omit_positions_on_all.json");

src/test/java/org/elasticsearch/test/unit/index/mapper/all/mapping.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@
1515
},
1616
"last":{
1717
"type":"string",
18-
"index":"not_analyzed"
18+
"index":"not_analyzed",
19+
"boost":2.0
1920
}
2021
}
2122
},
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
{
2+
"person":{
3+
"_all":{
4+
"enabled":true
5+
},
6+
"properties":{
7+
"name":{
8+
"type":"object",
9+
"dynamic":false,
10+
"properties":{
11+
"first":{
12+
"type":"string",
13+
"store":"yes",
14+
"include_in_all":false
15+
},
16+
"last":{
17+
"type":"string",
18+
"index":"not_analyzed"
19+
}
20+
}
21+
},
22+
"address":{
23+
"type":"object",
24+
"include_in_all":false,
25+
"properties":{
26+
"first":{
27+
"properties":{
28+
"location":{
29+
"type":"string",
30+
"store":"yes",
31+
"index_name":"firstLocation"
32+
}
33+
}
34+
},
35+
"last":{
36+
"properties":{
37+
"location":{
38+
"type":"string",
39+
"include_in_all":true
40+
}
41+
}
42+
}
43+
}
44+
},
45+
"simple1":{
46+
"type":"long",
47+
"include_in_all":true
48+
},
49+
"simple2":{
50+
"type":"long",
51+
"include_in_all":false
52+
}
53+
}
54+
}
55+
}

src/test/java/org/elasticsearch/test/unit/index/mapper/all/store-mapping.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
},
1717
"last":{
1818
"type":"string",
19-
"index":"not_analyzed"
19+
"index":"not_analyzed",
20+
"boost":2.0
2021
}
2122
}
2223
},

0 commit comments

Comments
 (0)