From b2bddca4bbedd09ddd1323de10a15a0372e5fab5 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 9 May 2019 11:25:59 +0100 Subject: [PATCH 1/4] Simplify handling of keyword field normalizers --- .../subphase/highlight/AnnotatedTextHighlighter.java | 4 ++-- .../admin/indices/analyze/TransportAnalyzeAction.java | 9 +-------- .../elasticsearch/index/mapper/KeywordFieldMapper.java | 3 ++- .../index/termvectors/TermVectorsService.java | 8 +------- .../search/fetch/subphase/highlight/HighlightUtils.java | 7 ------- .../fetch/subphase/highlight/PlainHighlighter.java | 2 +- .../fetch/subphase/highlight/UnifiedHighlighter.java | 6 +++--- 7 files changed, 10 insertions(+), 29 deletions(-) diff --git a/plugins/mapper-annotated-text/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/AnnotatedTextHighlighter.java b/plugins/mapper-annotated-text/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/AnnotatedTextHighlighter.java index 2ba7838b90950..6b1a1c9254cf2 100644 --- a/plugins/mapper-annotated-text/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/AnnotatedTextHighlighter.java +++ b/plugins/mapper-annotated-text/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/AnnotatedTextHighlighter.java @@ -39,8 +39,8 @@ public class AnnotatedTextHighlighter extends UnifiedHighlighter { public static final String NAME = "annotated"; @Override - protected Analyzer getAnalyzer(DocumentMapper docMapper, MappedFieldType type, HitContext hitContext) { - return new AnnotatedHighlighterAnalyzer(super.getAnalyzer(docMapper, type, hitContext), hitContext); + protected Analyzer getAnalyzer(DocumentMapper docMapper, HitContext hitContext) { + return new AnnotatedHighlighterAnalyzer(super.getAnalyzer(docMapper, hitContext), hitContext); } // Convert the marked-up values held on-disk to plain-text versions for highlighting diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/analyze/TransportAnalyzeAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/analyze/TransportAnalyzeAction.java index 9538bd4b4d22c..4b975456019bc 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/analyze/TransportAnalyzeAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/analyze/TransportAnalyzeAction.java @@ -49,12 +49,11 @@ import org.elasticsearch.index.analysis.CharFilterFactory; import org.elasticsearch.index.analysis.CustomAnalyzer; import org.elasticsearch.index.analysis.IndexAnalyzers; +import org.elasticsearch.index.analysis.NamedAnalyzer; import org.elasticsearch.index.analysis.NormalizingCharFilterFactory; import org.elasticsearch.index.analysis.NormalizingTokenFilterFactory; -import org.elasticsearch.index.analysis.NamedAnalyzer; import org.elasticsearch.index.analysis.TokenFilterFactory; import org.elasticsearch.index.analysis.TokenizerFactory; -import org.elasticsearch.index.mapper.KeywordFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.indices.IndicesService; @@ -142,12 +141,6 @@ protected AnalyzeResponse shardOperation(AnalyzeRequest request, ShardId shardId if (fieldType != null) { if (fieldType.tokenized()) { analyzer = fieldType.indexAnalyzer(); - } else if (fieldType instanceof KeywordFieldMapper.KeywordFieldType) { - analyzer = ((KeywordFieldMapper.KeywordFieldType) fieldType).normalizer(); - if (analyzer == null) { - // this will be KeywordAnalyzer - analyzer = fieldType.indexAnalyzer(); - } } else { throw new IllegalArgumentException("Can't process field [" + request.field() + "], Analysis requests are only supported on tokenized fields"); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java index 20b4bb37cc7aa..099ae9b2aa7a2 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java @@ -235,13 +235,14 @@ public String typeName() { return CONTENT_TYPE; } - public NamedAnalyzer normalizer() { + private NamedAnalyzer normalizer() { return normalizer; } public void setNormalizer(NamedAnalyzer normalizer) { checkIfFrozen(); this.normalizer = normalizer; + setIndexAnalyzer(normalizer); } public boolean splitQueriesOnWhitespace() { diff --git a/server/src/main/java/org/elasticsearch/index/termvectors/TermVectorsService.java b/server/src/main/java/org/elasticsearch/index/termvectors/TermVectorsService.java index a05870b842f2d..b53c3d8da427c 100644 --- a/server/src/main/java/org/elasticsearch/index/termvectors/TermVectorsService.java +++ b/server/src/main/java/org/elasticsearch/index/termvectors/TermVectorsService.java @@ -44,7 +44,6 @@ import org.elasticsearch.index.get.GetResult; import org.elasticsearch.index.mapper.DocumentMapperForType; import org.elasticsearch.index.mapper.IdFieldMapper; -import org.elasticsearch.index.mapper.KeywordFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.ParseContext; @@ -235,12 +234,7 @@ private static Analyzer getAnalyzerAtField(IndexShard indexShard, String field, analyzer = mapperService.getIndexAnalyzers().get(perFieldAnalyzer.get(field).toString()); } else { MappedFieldType fieldType = mapperService.fullName(field); - if (fieldType instanceof KeywordFieldMapper.KeywordFieldType) { - KeywordFieldMapper.KeywordFieldType keywordFieldType = (KeywordFieldMapper.KeywordFieldType) fieldType; - analyzer = keywordFieldType.normalizer() == null ? keywordFieldType.indexAnalyzer() : keywordFieldType.normalizer(); - } else { - analyzer = fieldType.indexAnalyzer(); - } + analyzer = fieldType.indexAnalyzer(); } if (analyzer == null) { analyzer = mapperService.getIndexAnalyzers().getDefaultIndexAnalyzer(); diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightUtils.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightUtils.java index 6ae302ee87a25..b6b73c5873def 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightUtils.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightUtils.java @@ -24,7 +24,6 @@ import org.apache.lucene.search.highlight.SimpleHTMLEncoder; import org.elasticsearch.index.fieldvisitor.CustomFieldsVisitor; import org.elasticsearch.index.mapper.DocumentMapper; -import org.elasticsearch.index.mapper.KeywordFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.search.fetch.FetchSubPhase; import org.elasticsearch.search.internal.SearchContext; @@ -79,12 +78,6 @@ public static class Encoders { } static Analyzer getAnalyzer(DocumentMapper docMapper, MappedFieldType type) { - if (type instanceof KeywordFieldMapper.KeywordFieldType) { - KeywordFieldMapper.KeywordFieldType keywordFieldType = (KeywordFieldMapper.KeywordFieldType) type; - if (keywordFieldType.normalizer() != null) { - return keywordFieldType.normalizer(); - } - } return docMapper.mappers().indexAnalyzer(); } } diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/PlainHighlighter.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/PlainHighlighter.java index ec5071706b031..6ad155104a4cc 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/PlainHighlighter.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/PlainHighlighter.java @@ -101,7 +101,7 @@ public HighlightField highlight(HighlighterContext highlighterContext) { int numberOfFragments = field.fieldOptions().numberOfFragments() == 0 ? 1 : field.fieldOptions().numberOfFragments(); ArrayList fragsList = new ArrayList<>(); List textsToHighlight; - Analyzer analyzer = HighlightUtils.getAnalyzer(context.mapperService().documentMapper(hitContext.hit().getType()), fieldType); + Analyzer analyzer = context.mapperService().documentMapper(hitContext.hit().getType()).mappers().indexAnalyzer(); final int maxAnalyzedOffset = context.indexShard().indexSettings().getHighlightMaxAnalyzedOffset(); try { diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/UnifiedHighlighter.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/UnifiedHighlighter.java index 2a75e9c58f4fc..b806fb9cd312f 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/UnifiedHighlighter.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/UnifiedHighlighter.java @@ -70,7 +70,7 @@ public HighlightField highlight(HighlighterContext highlighterContext) { int numberOfFragments; try { - final Analyzer analyzer = getAnalyzer(context.mapperService().documentMapper(hitContext.hit().getType()), fieldType, + final Analyzer analyzer = getAnalyzer(context.mapperService().documentMapper(hitContext.hit().getType()), hitContext); List fieldValues = loadFieldValues(fieldType, field, context, hitContext); if (fieldValues.size() == 0) { @@ -150,8 +150,8 @@ protected PassageFormatter getPassageFormatter(HitContext hitContext, SearchCont } - protected Analyzer getAnalyzer(DocumentMapper docMapper, MappedFieldType type, HitContext hitContext) { - return HighlightUtils.getAnalyzer(docMapper, type); + protected Analyzer getAnalyzer(DocumentMapper docMapper, HitContext hitContext) { + return docMapper.mappers().indexAnalyzer(); } protected List loadFieldValues(MappedFieldType fieldType, SearchContextHighlight.Field field, SearchContext context, From 2ef746f500024cb88596769fb944cd8bf779dd12 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 9 May 2019 13:04:48 +0100 Subject: [PATCH 2/4] fix expected error message --- .../elasticsearch/index/mapper/KeywordFieldMapperTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java index dd7cb17ef127c..1bdf40bcc6708 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java @@ -401,7 +401,8 @@ public void testUpdateNormalizer() throws IOException { () -> indexService.mapperService().merge("type", new CompressedXContent(mapping2), MergeReason.MAPPING_UPDATE)); assertEquals( - "Mapper for [field] conflicts with existing mapping:\n[mapper [field] has different [normalizer]]", + "Mapper for [field] conflicts with existing mapping:\n" + + "[mapper [field] has different [analyzer], mapper [field] has different [normalizer]]", e.getMessage()); } From 3c60eb06f3b35061ec758b9b4282fa2974ff0f1e Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Fri, 10 May 2019 08:55:23 +0100 Subject: [PATCH 3/4] Reinstate one instanceof check --- .../action/admin/indices/analyze/TransportAnalyzeAction.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/analyze/TransportAnalyzeAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/analyze/TransportAnalyzeAction.java index 4b975456019bc..5025c01f266aa 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/analyze/TransportAnalyzeAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/analyze/TransportAnalyzeAction.java @@ -54,6 +54,7 @@ import org.elasticsearch.index.analysis.NormalizingTokenFilterFactory; import org.elasticsearch.index.analysis.TokenFilterFactory; import org.elasticsearch.index.analysis.TokenizerFactory; +import org.elasticsearch.index.mapper.KeywordFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.indices.IndicesService; @@ -139,7 +140,7 @@ protected AnalyzeResponse shardOperation(AnalyzeRequest request, ShardId shardId } MappedFieldType fieldType = indexService.mapperService().fullName(request.field()); if (fieldType != null) { - if (fieldType.tokenized()) { + if (fieldType.tokenized() || fieldType instanceof KeywordFieldMapper.KeywordFieldType) { analyzer = fieldType.indexAnalyzer(); } else { throw new IllegalArgumentException("Can't process field [" + request.field() + From 8354d929188e9f323b595e69aee38ee4101f4ab4 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Fri, 10 May 2019 12:15:54 +0100 Subject: [PATCH 4/4] Remove unused method --- .../search/fetch/subphase/highlight/HighlightUtils.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightUtils.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightUtils.java index b6b73c5873def..e9ec29cef4ae7 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightUtils.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightUtils.java @@ -18,12 +18,10 @@ */ package org.elasticsearch.search.fetch.subphase.highlight; -import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.search.highlight.DefaultEncoder; import org.apache.lucene.search.highlight.Encoder; import org.apache.lucene.search.highlight.SimpleHTMLEncoder; import org.elasticsearch.index.fieldvisitor.CustomFieldsVisitor; -import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.search.fetch.FetchSubPhase; import org.elasticsearch.search.internal.SearchContext; @@ -77,7 +75,4 @@ public static class Encoders { public static final Encoder HTML = new SimpleHTMLEncoder(); } - static Analyzer getAnalyzer(DocumentMapper docMapper, MappedFieldType type) { - return docMapper.mappers().indexAnalyzer(); - } }