Skip to content

Commit

Permalink
Annotated text type should extend TextFieldType (#49555)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
romseygeek committed Nov 26, 2019
1 parent b5d7c93 commit fe2c651
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,29 +31,19 @@
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;
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;
Expand Down Expand Up @@ -531,7 +521,7 @@ private void emitAnnotation(int firstSpannedTextPosInc, int annotationPosLen) th
}


public static final class AnnotatedTextFieldType extends StringFieldType {
public static final class AnnotatedTextFieldType extends TextFieldMapper.TextFieldType {

public AnnotatedTextFieldType() {
setTokenized(true);
Expand Down Expand Up @@ -562,37 +552,6 @@ public String typeName() {
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,4 @@ public void testEmptyName() throws IOException {
assertThat(e.getMessage(), containsString("name cannot be empty string"));
}



}
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ public String toString() {
}
}

public static final class TextFieldType extends StringFieldType {
public static class TextFieldType extends StringFieldType {

private boolean fielddata;
private double fielddataMinFrequency;
Expand Down

0 comments on commit fe2c651

Please sign in to comment.