percolator: The percolate api shouldn't add field mappings for unmapped fields inside the document being percolated to the mapping.

Closes #15751
This commit is contained in:
Martijn van Groningen 2016-01-19 09:35:48 +01:00
parent 4c1e93bd89
commit 8b02f214c4
9 changed files with 29 additions and 112 deletions

View File

@ -39,6 +39,8 @@ import org.apache.lucene.util.CloseableThreadLocal;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.lucene.search.Queries;
import org.elasticsearch.index.engine.Engine;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.ParseContext;
import org.elasticsearch.index.mapper.ParsedDocument;
import org.elasticsearch.index.mapper.internal.UidFieldMapper;
@ -76,8 +78,7 @@ class MultiDocumentPercolatorIndex implements PercolatorIndex {
} else {
memoryIndex = new MemoryIndex(true);
}
Analyzer analyzer = context.mapperService().documentMapper(parsedDocument.type()).mappers().indexAnalyzer();
memoryIndices[i] = indexDoc(d, analyzer, memoryIndex).createSearcher().getIndexReader();
memoryIndices[i] = indexDoc(d, memoryIndex, context, parsedDocument).createSearcher().getIndexReader();
}
try {
MultiReader mReader = new MultiReader(memoryIndices, true);
@ -101,8 +102,13 @@ class MultiDocumentPercolatorIndex implements PercolatorIndex {
}
}
MemoryIndex indexDoc(ParseContext.Document d, Analyzer analyzer, MemoryIndex memoryIndex) {
MemoryIndex indexDoc(ParseContext.Document d, MemoryIndex memoryIndex, PercolateContext context, ParsedDocument parsedDocument) {
for (IndexableField field : d.getFields()) {
Analyzer analyzer = context.analysisService().defaultIndexAnalyzer();
DocumentMapper documentMapper = context.mapperService().documentMapper(parsedDocument.type());
if (documentMapper != null && documentMapper.mappers().getMapper(field.name()) != null) {
analyzer = documentMapper.mappers().indexAnalyzer();
}
if (field.fieldType().indexOptions() == IndexOptions.NONE && field.name().equals(UidFieldMapper.NAME)) {
continue;
}

View File

@ -49,14 +49,13 @@ public class PercolateDocumentParser {
private final HighlightPhase highlightPhase;
private final SortParseElement sortParseElement;
private final AggregationPhase aggregationPhase;
private final MappingUpdatedAction mappingUpdatedAction;
@Inject
public PercolateDocumentParser(HighlightPhase highlightPhase, SortParseElement sortParseElement, AggregationPhase aggregationPhase, MappingUpdatedAction mappingUpdatedAction) {
public PercolateDocumentParser(HighlightPhase highlightPhase, SortParseElement sortParseElement,
AggregationPhase aggregationPhase) {
this.highlightPhase = highlightPhase;
this.sortParseElement = sortParseElement;
this.aggregationPhase = aggregationPhase;
this.mappingUpdatedAction = mappingUpdatedAction;
}
public ParsedDocument parse(PercolateShardRequest request, PercolateContext context, MapperService mapperService, QueryShardContext queryShardContext) {
@ -98,9 +97,6 @@ public class PercolateDocumentParser {
if (docMapper.getMapping() != null) {
doc.addDynamicMappingsUpdate(docMapper.getMapping());
}
if (doc.dynamicMappingsUpdate() != null) {
mappingUpdatedAction.updateMappingOnMasterSynchronously(request.shardId().getIndex(), request.documentType(), doc.dynamicMappingsUpdate());
}
// the document parsing exists the "doc" object, so we need to set the new current field.
currentFieldName = parser.currentName();
}

View File

@ -52,6 +52,7 @@ import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.common.util.BigArrays;
import org.elasticsearch.index.IndexService;
import org.elasticsearch.index.fieldvisitor.SingleFieldsVisitor;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.ParsedDocument;
import org.elasticsearch.index.mapper.internal.UidFieldMapper;
import org.elasticsearch.index.percolator.PercolatorFieldMapper;
@ -201,7 +202,8 @@ public class PercolatorService extends AbstractComponent {
// parse the source either into one MemoryIndex, if it is a single document or index multiple docs if nested
PercolatorIndex percolatorIndex;
boolean isNested = indexShard.mapperService().documentMapper(request.documentType()).hasNestedObjects();
DocumentMapper documentMapper = indexShard.mapperService().documentMapper(request.documentType());
boolean isNested = documentMapper != null && documentMapper.hasNestedObjects();
if (parsedDocument.docs().size() > 1) {
assert isNested;
percolatorIndex = multi;

View File

@ -28,6 +28,7 @@ import org.apache.lucene.index.memory.MemoryIndex;
import org.apache.lucene.util.CloseableThreadLocal;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.index.engine.Engine;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.ParsedDocument;
import org.elasticsearch.index.mapper.internal.UidFieldMapper;
@ -49,11 +50,15 @@ class SingleDocumentPercolatorIndex implements PercolatorIndex {
public void prepare(PercolateContext context, ParsedDocument parsedDocument) {
MemoryIndex memoryIndex = cache.get();
for (IndexableField field : parsedDocument.rootDoc().getFields()) {
Analyzer analyzer = context.analysisService().defaultIndexAnalyzer();
DocumentMapper documentMapper = context.mapperService().documentMapper(parsedDocument.type());
if (documentMapper != null && documentMapper.mappers().getMapper(field.name()) != null) {
analyzer = documentMapper.mappers().indexAnalyzer();
}
if (field.fieldType().indexOptions() == IndexOptions.NONE && field.name().equals(UidFieldMapper.NAME)) {
continue;
}
try {
Analyzer analyzer = context.mapperService().documentMapper(parsedDocument.type()).mappers().indexAnalyzer();
// TODO: instead of passing null here, we can have a CTL<Map<String,TokenStream>> and pass previous,
// like the indexer does
try (TokenStream tokenStream = field.tokenStream(analyzer, null)) {

View File

@ -23,7 +23,6 @@ import org.apache.lucene.index.Term;
import org.apache.lucene.search.TermQuery;
import org.elasticsearch.Version;
import org.elasticsearch.action.percolate.PercolateShardRequest;
import org.elasticsearch.cluster.action.index.MappingUpdatedAction;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentBuilder;
@ -90,10 +89,7 @@ public class PercolateDocumentParserTests extends ESTestCase {
HighlightPhase highlightPhase = new HighlightPhase(Settings.EMPTY, new Highlighters());
AggregatorParsers aggregatorParsers = new AggregatorParsers(Collections.emptySet(), Collections.emptySet());
AggregationPhase aggregationPhase = new AggregationPhase(new AggregationParseElement(aggregatorParsers), new AggregationBinaryParseElement(aggregatorParsers));
MappingUpdatedAction mappingUpdatedAction = Mockito.mock(MappingUpdatedAction.class);
parser = new PercolateDocumentParser(
highlightPhase, new SortParseElement(), aggregationPhase, mappingUpdatedAction
);
parser = new PercolateDocumentParser(highlightPhase, new SortParseElement(), aggregationPhase);
request = Mockito.mock(PercolateShardRequest.class);
Mockito.when(request.shardId()).thenReturn(new ShardId(new Index("_index"), 0));

View File

@ -175,7 +175,7 @@ public class PercolatorIT extends ESIntegTestCase {
}
public void testSimple2() throws Exception {
assertAcked(prepareCreate("test").addMapping("type1", "field1", "type=long,doc_values=true"));
assertAcked(prepareCreate("test").addMapping("type1", "field1", "type=long,doc_values=true", "field2", "type=string"));
ensureGreen();
// introduce the doc
@ -1577,92 +1577,6 @@ public class PercolatorIT extends ESIntegTestCase {
assertEquals(response.getMatches()[0].getId().string(), "Q");
}
public void testPercolationWithDynamicTemplates() throws Exception {
assertAcked(prepareCreate("idx").addMapping("type", jsonBuilder().startObject().startObject("type")
.field("dynamic", false)
.startObject("properties")
.startObject("custom")
.field("dynamic", true)
.field("type", "object")
.field("include_in_all", false)
.endObject()
.endObject()
.startArray("dynamic_templates")
.startObject()
.startObject("custom_fields")
.field("path_match", "custom.*")
.startObject("mapping")
.field("index", "not_analyzed")
.endObject()
.endObject()
.endObject()
.endArray()
.endObject().endObject()));
ensureGreen("idx");
try {
client().prepareIndex("idx", PercolatorService.TYPE_NAME, "1")
.setSource(jsonBuilder().startObject().field("query", QueryBuilders.queryStringQuery("color:red")).endObject())
.get();
fail();
} catch (MapperParsingException e) {
}
refresh();
PercolateResponse percolateResponse = client().preparePercolate().setDocumentType("type")
.setPercolateDoc(new PercolateSourceBuilder.DocBuilder().setDoc(jsonBuilder().startObject().startObject("custom").field("color", "blue").endObject().endObject()))
.get();
assertMatchCount(percolateResponse, 0l);
assertThat(percolateResponse.getMatches(), arrayWithSize(0));
// The previous percolate request introduced the custom.color field, so now we register the query again
// and the field name `color` will be resolved to `custom.color` field in mapping via smart field mapping resolving.
client().prepareIndex("idx", PercolatorService.TYPE_NAME, "1")
.setSource(jsonBuilder().startObject().field("query", QueryBuilders.queryStringQuery("custom.color:red")).endObject())
.get();
client().prepareIndex("idx", PercolatorService.TYPE_NAME, "2")
.setSource(jsonBuilder().startObject().field("query", QueryBuilders.queryStringQuery("custom.color:blue")).field("type", "type").endObject())
.get();
refresh();
// The second request will yield a match, since the query during the proper field during parsing.
percolateResponse = client().preparePercolate().setDocumentType("type")
.setPercolateDoc(new PercolateSourceBuilder.DocBuilder().setDoc(jsonBuilder().startObject().startObject("custom").field("color", "blue").endObject().endObject()))
.get();
assertMatchCount(percolateResponse, 1l);
assertThat(percolateResponse.getMatches()[0].getId().string(), equalTo("2"));
}
public void testUpdateMappingDynamicallyWhilePercolating() throws Exception {
createIndex("test");
ensureSearchable();
// percolation source
XContentBuilder percolateDocumentSource = XContentFactory.jsonBuilder().startObject().startObject("doc")
.field("field1", 1)
.field("field2", "value")
.endObject().endObject();
PercolateResponse response = client().preparePercolate()
.setIndices("test").setDocumentType("type1")
.setSource(percolateDocumentSource).execute().actionGet();
assertAllSuccessful(response);
assertMatchCount(response, 0l);
assertThat(response.getMatches(), arrayWithSize(0));
assertMappingOnMaster("test", "type1");
GetMappingsResponse mappingsResponse = client().admin().indices().prepareGetMappings("test").get();
assertThat(mappingsResponse.getMappings().get("test"), notNullValue());
assertThat(mappingsResponse.getMappings().get("test").get("type1"), notNullValue());
assertThat(mappingsResponse.getMappings().get("test").get("type1").getSourceAsMap().isEmpty(), is(false));
Map<String, Object> properties = (Map<String, Object>) mappingsResponse.getMappings().get("test").get("type1").getSourceAsMap().get("properties");
assertThat(((Map<String, String>) properties.get("field1")).get("type"), equalTo("long"));
assertThat(((Map<String, String>) properties.get("field2")).get("type"), equalTo("string"));
}
public void testDontReportDeletedPercolatorDocs() throws Exception {
client().admin().indices().prepareCreate("test").execute().actionGet();
ensureGreen();

View File

@ -644,6 +644,10 @@ The percolate api can no longer accept documents that have fields that don't exi
When percolating an existing document then specifying a document in the source of the percolate request is not allowed
any more.
The percolate api no longer modifies the mappings. Before the percolate api could be used to dynamically introduce new
fields to the mappings based on the fields in the document being percolated. This no longer works, because these
unmapped fields are not persisted in the mapping.
Percolator documents are no longer excluded from the search response.
[[breaking_30_packaging]]

View File

@ -20,14 +20,8 @@ in a request to the percolate API.
=====================================
Fields referred to in a percolator query must *already* exist in the mapping
associated with the index used for percolation.
There are two ways to make sure that a field mapping exist:
* Add or update a mapping via the <<indices-create-index,create index>> or
<<indices-put-mapping,put mapping>> APIs.
* Percolate a document before registering a query. Percolating a document can
add field mappings dynamically, in the same way as happens when indexing a
document.
associated with the index used for percolation. In order to make sure these fields exist,
add or update a mapping via the <<indices-create-index,create index>> or <<indices-put-mapping,put mapping>> APIs.
=====================================

View File

@ -27,7 +27,7 @@
- do:
percolate:
index: test_index
type: test_type
type: type_1
body:
doc:
foo: "bar foo"