From fe2c65185ea0c89f3a4bbc1f662fccf6c726aa81 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 26 Nov 2019 16:51:41 +0000 Subject: [PATCH] Annotated text type should extend TextFieldType (#49555) The annotated text mapper has a field type that currently extends StringFieldType, which means that all the positional-related query factory methods need to be copied over from TextFieldType. In addition, MappedFieldType.intervals() hasn't been overridden, so you can't use intervals queries with annotated text - a major drawback, since one of the purposes of annotated text is to be able to run positional queries against annotations. This commit changes the annotated text field type to extend TextFieldType instead, adding tests to ensure that position queries work correctly. Closes #49289 --- .../AnnotatedTextFieldMapper.java | 43 +----------------- .../AnnotatedTextFieldMapperTests.java | 2 - .../AnnotatedTextFieldTypeTests.java | 44 +++++++++++++++++++ .../test/mapper_annotatedtext/10_basic.yml | 39 +++++++++++++++- .../index/mapper/TextFieldMapper.java | 2 +- 5 files changed, 83 insertions(+), 47 deletions(-) create mode 100644 plugins/mapper-annotated-text/src/test/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldTypeTests.java diff --git a/plugins/mapper-annotated-text/src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapper.java b/plugins/mapper-annotated-text/src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapper.java index 7b195bdc7b4..60036511e2f 100644 --- a/plugins/mapper-annotated-text/src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapper.java +++ b/plugins/mapper-annotated-text/src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapper.java @@ -31,13 +31,6 @@ import org.apache.lucene.analysis.tokenattributes.TypeAttribute; import org.apache.lucene.document.Field; import org.apache.lucene.index.IndexOptions; import org.apache.lucene.index.IndexableField; -import org.apache.lucene.index.Term; -import org.apache.lucene.search.NormsFieldExistsQuery; -import org.apache.lucene.search.PrefixQuery; -import org.apache.lucene.search.Query; -import org.apache.lucene.search.TermQuery; -import org.apache.lucene.search.spans.SpanMultiTermQueryWrapper; -import org.apache.lucene.search.spans.SpanQuery; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -45,15 +38,12 @@ import org.elasticsearch.common.xcontent.support.XContentMapValues; import org.elasticsearch.index.analysis.AnalyzerScope; import org.elasticsearch.index.analysis.NamedAnalyzer; import org.elasticsearch.index.mapper.FieldMapper; -import org.elasticsearch.index.mapper.FieldNamesFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.Mapper; import org.elasticsearch.index.mapper.MapperParsingException; import org.elasticsearch.index.mapper.ParseContext; -import org.elasticsearch.index.mapper.StringFieldType; import org.elasticsearch.index.mapper.TextFieldMapper; import org.elasticsearch.index.mapper.annotatedtext.AnnotatedTextFieldMapper.AnnotatedText.AnnotationToken; -import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.search.fetch.FetchSubPhase.HitContext; import java.io.IOException; @@ -531,7 +521,7 @@ public class AnnotatedTextFieldMapper extends FieldMapper { } - public static final class AnnotatedTextFieldType extends StringFieldType { + public static final class AnnotatedTextFieldType extends TextFieldMapper.TextFieldType { public AnnotatedTextFieldType() { setTokenized(true); @@ -562,37 +552,6 @@ public class AnnotatedTextFieldMapper extends FieldMapper { return CONTENT_TYPE; } - @Override - public Query existsQuery(QueryShardContext context) { - if (omitNorms()) { - return new TermQuery(new Term(FieldNamesFieldMapper.NAME, name())); - } else { - return new NormsFieldExistsQuery(name()); - } - } - - @Override - public SpanQuery spanPrefixQuery(String value, SpanMultiTermQueryWrapper.SpanRewriteMethod method, QueryShardContext context) { - SpanMultiTermQueryWrapper spanMulti = - new SpanMultiTermQueryWrapper<>(new PrefixQuery(new Term(name(), indexedValueForSearch(value)))); - spanMulti.setRewriteMethod(method); - return spanMulti; - } - - @Override - public Query phraseQuery(TokenStream stream, int slop, boolean enablePositionIncrements) throws IOException { - return TextFieldMapper.createPhraseQuery(stream, name(), slop, enablePositionIncrements); - } - - @Override - public Query multiPhraseQuery(TokenStream stream, int slop, boolean enablePositionIncrements) throws IOException { - return TextFieldMapper.createPhraseQuery(stream, name(), slop, enablePositionIncrements); - } - - @Override - public Query phrasePrefixQuery(TokenStream stream, int slop, int maxExpansions) throws IOException { - return TextFieldMapper.createPhrasePrefixQuery(stream, name(), slop, maxExpansions, null, null); - } } private int positionIncrementGap; diff --git a/plugins/mapper-annotated-text/src/test/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapperTests.java b/plugins/mapper-annotated-text/src/test/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapperTests.java index aa0dbe07c83..12251911bf7 100644 --- a/plugins/mapper-annotated-text/src/test/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapperTests.java +++ b/plugins/mapper-annotated-text/src/test/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapperTests.java @@ -672,6 +672,4 @@ public class AnnotatedTextFieldMapperTests extends ESSingleNodeTestCase { assertThat(e.getMessage(), containsString("name cannot be empty string")); } - - } diff --git a/plugins/mapper-annotated-text/src/test/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldTypeTests.java b/plugins/mapper-annotated-text/src/test/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldTypeTests.java new file mode 100644 index 00000000000..0bbc25e6171 --- /dev/null +++ b/plugins/mapper-annotated-text/src/test/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldTypeTests.java @@ -0,0 +1,44 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.mapper.annotatedtext; + +import org.apache.lucene.analysis.standard.StandardAnalyzer; +import org.apache.lucene.queries.intervals.Intervals; +import org.apache.lucene.queries.intervals.IntervalsSource; +import org.elasticsearch.index.analysis.AnalyzerScope; +import org.elasticsearch.index.analysis.NamedAnalyzer; +import org.elasticsearch.index.mapper.FieldTypeTestCase; +import org.elasticsearch.index.mapper.MappedFieldType; + +import java.io.IOException; + +public class AnnotatedTextFieldTypeTests extends FieldTypeTestCase { + @Override + protected MappedFieldType createDefaultFieldType() { + return new AnnotatedTextFieldMapper.AnnotatedTextFieldType(); + } + + public void testIntervals() throws IOException { + MappedFieldType ft = createDefaultFieldType(); + NamedAnalyzer a = new NamedAnalyzer("name", AnalyzerScope.INDEX, new StandardAnalyzer()); + IntervalsSource source = ft.intervals("Donald Trump", 0, true, a, false); + assertEquals(Intervals.phrase(Intervals.term("donald"), Intervals.term("trump")), source); + } +} diff --git a/plugins/mapper-annotated-text/src/test/resources/rest-api-spec/test/mapper_annotatedtext/10_basic.yml b/plugins/mapper-annotated-text/src/test/resources/rest-api-spec/test/mapper_annotatedtext/10_basic.yml index 63516516252..be3b32e6338 100644 --- a/plugins/mapper-annotated-text/src/test/resources/rest-api-spec/test/mapper_annotatedtext/10_basic.yml +++ b/plugins/mapper-annotated-text/src/test/resources/rest-api-spec/test/mapper_annotatedtext/10_basic.yml @@ -43,6 +43,41 @@ - match: {hits.hits.0.highlight.text.0: "The [quick](_hit_term=quick) brown fox is brown."} + - do: + search: + body: + query: + intervals: + text: + match: + query: entity_3789 brown + + - match: { hits.total.value: 1 } + + - do: + search: + body: + query: + span_near: + clauses: [ + span_term: { text: entity_3789 }, + span_term: { text: brown } + ] + in_order: true + slop: 10 + + - match: { hits.total.value: 1 } + + - do: + search: + body: + query: + match_phrase: + text: "fox is brown" + + - match: { hits.total.value: 1 } + + --- "issue 39395 thread safety issue -requires multiple calls to reveal": - skip: @@ -65,13 +100,13 @@ index: index: annotated id: 1 - body: + body: "my_field" : "[A](~MARK0&~MARK0) [B](~MARK1)" - do: index: index: annotated id: 2 - body: + body: "my_field" : "[A](~MARK0) [C](~MARK2)" refresh: true - do: diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java index 4c73dbe340c..53103f15fcf 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java @@ -519,7 +519,7 @@ public class TextFieldMapper extends FieldMapper { } } - public static final class TextFieldType extends StringFieldType { + public static class TextFieldType extends StringFieldType { private boolean fielddata; private double fielddataMinFrequency;