From 2faea54a91cdf3fc352b310a334e16f500ac04cf Mon Sep 17 00:00:00 2001 From: Svilen Mihaylov Date: Tue, 17 Dec 2024 16:44:20 -0500 Subject: [PATCH 01/18] Add ability to set "max_analyzed_offet" implicitly to "index.highlight .max_analyzed_offset", by setting it excplicitly to "-1". --- .../AnnotatedTextHighlighter.java | 3 +- .../AnnotatedTextHighlighterTests.java | 19 ++++++++++-- .../uhighlight/CustomFieldHighlighter.java | 6 ++-- .../uhighlight/CustomUnifiedHighlighter.java | 8 ++--- .../uhighlight/QueryMaxAnalyzedOffset.java | 30 +++++++++++++++++++ .../highlight/DefaultHighlighter.java | 12 +++++--- .../subphase/highlight/PlainHighlighter.java | 13 +++++--- .../CustomUnifiedHighlighterTests.java | 2 +- 8 files changed, 74 insertions(+), 19 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/lucene/search/uhighlight/QueryMaxAnalyzedOffset.java diff --git a/plugins/mapper-annotated-text/src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextHighlighter.java b/plugins/mapper-annotated-text/src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextHighlighter.java index 8b4a9d6544b75..f636335701da5 100644 --- a/plugins/mapper-annotated-text/src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextHighlighter.java +++ b/plugins/mapper-annotated-text/src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextHighlighter.java @@ -17,6 +17,7 @@ import org.elasticsearch.index.mapper.annotatedtext.AnnotatedTextFieldMapper.AnnotatedText; import org.elasticsearch.index.query.SearchExecutionContext; import org.elasticsearch.lucene.search.uhighlight.CustomUnifiedHighlighter; +import org.elasticsearch.lucene.search.uhighlight.QueryMaxAnalyzedOffset; import org.elasticsearch.search.fetch.FetchSubPhase.HitContext; import org.elasticsearch.search.fetch.subphase.highlight.DefaultHighlighter; import org.elasticsearch.search.fetch.subphase.highlight.SearchHighlightContext; @@ -52,7 +53,7 @@ protected List loadFieldValues( } @Override - protected Analyzer wrapAnalyzer(Analyzer analyzer, Integer maxAnalyzedOffset) { + protected Analyzer wrapAnalyzer(Analyzer analyzer, QueryMaxAnalyzedOffset maxAnalyzedOffset) { return new AnnotatedHighlighterAnalyzer(super.wrapAnalyzer(analyzer, maxAnalyzedOffset)); } diff --git a/plugins/mapper-annotated-text/src/test/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextHighlighterTests.java b/plugins/mapper-annotated-text/src/test/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextHighlighterTests.java index d4c4ccfaa442d..53bb87bdf0acb 100644 --- a/plugins/mapper-annotated-text/src/test/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextHighlighterTests.java +++ b/plugins/mapper-annotated-text/src/test/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextHighlighterTests.java @@ -39,6 +39,7 @@ import org.elasticsearch.index.mapper.annotatedtext.AnnotatedTextFieldMapper.AnnotatedText; import org.elasticsearch.index.mapper.annotatedtext.AnnotatedTextFieldMapper.AnnotationAnalyzerWrapper; import org.elasticsearch.lucene.search.uhighlight.CustomUnifiedHighlighter; +import org.elasticsearch.lucene.search.uhighlight.QueryMaxAnalyzedOffset; import org.elasticsearch.lucene.search.uhighlight.Snippet; import org.elasticsearch.search.fetch.subphase.highlight.LimitTokenOffsetAnalyzer; import org.elasticsearch.test.ESTestCase; @@ -85,7 +86,7 @@ private void assertHighlightOneDoc( int noMatchSize, String[] expectedPassages, int maxAnalyzedOffset, - Integer queryMaxAnalyzedOffset + Integer queryMaxAnalyzedOffsetIn ) throws Exception { try (Directory dir = newDirectory()) { @@ -116,8 +117,9 @@ private void assertHighlightOneDoc( for (int i = 0; i < markedUpInputs.length; i++) { annotations[i] = AnnotatedText.parse(markedUpInputs[i]); } + QueryMaxAnalyzedOffset queryMaxAnalyzedOffset = QueryMaxAnalyzedOffset.create(queryMaxAnalyzedOffsetIn, maxAnalyzedOffset); if (queryMaxAnalyzedOffset != null) { - wrapperAnalyzer = new LimitTokenOffsetAnalyzer(wrapperAnalyzer, queryMaxAnalyzedOffset); + wrapperAnalyzer = new LimitTokenOffsetAnalyzer(wrapperAnalyzer, queryMaxAnalyzedOffset.getNotNull()); } AnnotatedHighlighterAnalyzer hiliteAnalyzer = new AnnotatedHighlighterAnalyzer(wrapperAnalyzer); hiliteAnalyzer.setAnnotations(annotations); @@ -311,6 +313,19 @@ public void testExceedMaxAnalyzedOffset() throws Exception { e.getMessage() ); + // Same as before, but force using index maxOffset (20) as queryMaxOffset by passing -1. + assertHighlightOneDoc( + "text", + new String[] { "[Long Text exceeds](Long+Text+exceeds) MAX analyzed offset)" }, + query, + Locale.ROOT, + breakIterator, + 0, + new String[] { "Long Text [exceeds](_hit_term=exceeds) MAX analyzed offset)" }, + 20, + -1 + ); + assertHighlightOneDoc( "text", new String[] { "[Long Text Exceeds](Long+Text+Exceeds) MAX analyzed offset [Long Text Exceeds](Long+Text+Exceeds)" }, diff --git a/server/src/main/java/org/elasticsearch/lucene/search/uhighlight/CustomFieldHighlighter.java b/server/src/main/java/org/elasticsearch/lucene/search/uhighlight/CustomFieldHighlighter.java index 3e59814de0585..acf186faf20b2 100644 --- a/server/src/main/java/org/elasticsearch/lucene/search/uhighlight/CustomFieldHighlighter.java +++ b/server/src/main/java/org/elasticsearch/lucene/search/uhighlight/CustomFieldHighlighter.java @@ -34,7 +34,7 @@ class CustomFieldHighlighter extends FieldHighlighter { private final Locale breakIteratorLocale; private final int noMatchSize; private String fieldValue; - private final Integer queryMaxAnalyzedOffset; + private final QueryMaxAnalyzedOffset queryMaxAnalyzedOffset; CustomFieldHighlighter( String field, @@ -47,7 +47,7 @@ class CustomFieldHighlighter extends FieldHighlighter { PassageFormatter passageFormatter, Comparator passageSortComparator, int noMatchSize, - Integer queryMaxAnalyzedOffset + QueryMaxAnalyzedOffset queryMaxAnalyzedOffset ) { super( field, @@ -113,7 +113,7 @@ protected Passage[] getSummaryPassagesNoHighlight(int maxPassages) { @Override protected Passage[] highlightOffsetsEnums(OffsetsEnum off) throws IOException { if (queryMaxAnalyzedOffset != null) { - off = new LimitedOffsetsEnum(off, queryMaxAnalyzedOffset); + off = new LimitedOffsetsEnum(off, queryMaxAnalyzedOffset.getNotNull()); } return super.highlightOffsetsEnums(off); } diff --git a/server/src/main/java/org/elasticsearch/lucene/search/uhighlight/CustomUnifiedHighlighter.java b/server/src/main/java/org/elasticsearch/lucene/search/uhighlight/CustomUnifiedHighlighter.java index d1c7d0415ad15..59dffb73985ac 100644 --- a/server/src/main/java/org/elasticsearch/lucene/search/uhighlight/CustomUnifiedHighlighter.java +++ b/server/src/main/java/org/elasticsearch/lucene/search/uhighlight/CustomUnifiedHighlighter.java @@ -66,7 +66,7 @@ public final class CustomUnifiedHighlighter extends UnifiedHighlighter { private final int noMatchSize; private final CustomFieldHighlighter fieldHighlighter; private final int maxAnalyzedOffset; - private final Integer queryMaxAnalyzedOffset; + private final QueryMaxAnalyzedOffset queryMaxAnalyzedOffset; /** * Creates a new instance of {@link CustomUnifiedHighlighter} @@ -94,7 +94,7 @@ public CustomUnifiedHighlighter( int noMatchSize, int maxPassages, int maxAnalyzedOffset, - Integer queryMaxAnalyzedOffset, + QueryMaxAnalyzedOffset queryMaxAnalyzedOffset, boolean requireFieldMatch, boolean weightMatchesEnabled ) { @@ -125,9 +125,9 @@ public Snippet[] highlightField(LeafReader reader, int docId, CheckedSupplier maxAnalyzedOffset) + if ((queryMaxAnalyzedOffset == null || queryMaxAnalyzedOffset.getNotNull() > maxAnalyzedOffset) && (getOffsetSource(field) == OffsetSource.ANALYSIS) - && (fieldValueLength > maxAnalyzedOffset))) { + && (fieldValueLength > maxAnalyzedOffset)) { throw new IllegalArgumentException( "The length [" + fieldValueLength diff --git a/server/src/main/java/org/elasticsearch/lucene/search/uhighlight/QueryMaxAnalyzedOffset.java b/server/src/main/java/org/elasticsearch/lucene/search/uhighlight/QueryMaxAnalyzedOffset.java new file mode 100644 index 0000000000000..e74b11d4e1a91 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/lucene/search/uhighlight/QueryMaxAnalyzedOffset.java @@ -0,0 +1,30 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.lucene.search.uhighlight; + +public class QueryMaxAnalyzedOffset { + private final int queryMaxAnalyzedOffset; + + private QueryMaxAnalyzedOffset(final int queryMaxAnalyzedOffset) { + // If we have a negative value, grab value for the actual maximum from the index. + this.queryMaxAnalyzedOffset = queryMaxAnalyzedOffset; + } + + public static QueryMaxAnalyzedOffset create(final Integer queryMaxAnalyzedOffset, final int indexMaxAnalyzedOffset) { + if (queryMaxAnalyzedOffset == null) { + return null; + } + return new QueryMaxAnalyzedOffset(queryMaxAnalyzedOffset < 0 ? indexMaxAnalyzedOffset : queryMaxAnalyzedOffset); + } + + public int getNotNull() { + return queryMaxAnalyzedOffset; + } +} diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/DefaultHighlighter.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/DefaultHighlighter.java index 75f8e5588761a..62a6e4b8dae84 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/DefaultHighlighter.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/DefaultHighlighter.java @@ -31,6 +31,7 @@ import org.elasticsearch.lucene.search.uhighlight.BoundedBreakIteratorScanner; import org.elasticsearch.lucene.search.uhighlight.CustomPassageFormatter; import org.elasticsearch.lucene.search.uhighlight.CustomUnifiedHighlighter; +import org.elasticsearch.lucene.search.uhighlight.QueryMaxAnalyzedOffset; import org.elasticsearch.lucene.search.uhighlight.Snippet; import org.elasticsearch.search.fetch.FetchContext; import org.elasticsearch.search.fetch.FetchSubPhase; @@ -121,7 +122,10 @@ CustomUnifiedHighlighter buildHighlighter(FieldHighlightContext fieldContext) { int maxAnalyzedOffset = indexSettings.getHighlightMaxAnalyzedOffset(); boolean weightMatchesEnabled = indexSettings.isWeightMatchesEnabled(); int numberOfFragments = fieldContext.field.fieldOptions().numberOfFragments(); - Integer queryMaxAnalyzedOffset = fieldContext.field.fieldOptions().maxAnalyzedOffset(); + QueryMaxAnalyzedOffset queryMaxAnalyzedOffset = QueryMaxAnalyzedOffset.create( + fieldContext.field.fieldOptions().maxAnalyzedOffset(), + maxAnalyzedOffset + ); Analyzer analyzer = wrapAnalyzer( fieldContext.context.getSearchExecutionContext().getIndexAnalyzer(f -> Lucene.KEYWORD_ANALYZER), queryMaxAnalyzedOffset @@ -171,7 +175,7 @@ CustomUnifiedHighlighter buildHighlighter(FieldHighlightContext fieldContext) { fieldContext.field.fieldOptions().noMatchSize(), highlighterNumberOfFragments, maxAnalyzedOffset, - fieldContext.field.fieldOptions().maxAnalyzedOffset(), + queryMaxAnalyzedOffset, fieldContext.field.fieldOptions().requireFieldMatch(), weightMatchesEnabled ); @@ -186,9 +190,9 @@ protected PassageFormatter getPassageFormatter(SearchHighlightContext.Field fiel ); } - protected Analyzer wrapAnalyzer(Analyzer analyzer, Integer maxAnalyzedOffset) { + protected Analyzer wrapAnalyzer(Analyzer analyzer, QueryMaxAnalyzedOffset maxAnalyzedOffset) { if (maxAnalyzedOffset != null) { - analyzer = new LimitTokenOffsetAnalyzer(analyzer, maxAnalyzedOffset); + analyzer = new LimitTokenOffsetAnalyzer(analyzer, maxAnalyzedOffset.getNotNull()); } return analyzer; } 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 3dd3dd1b42c8c..e1c09e925c1b4 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 @@ -27,6 +27,7 @@ import org.elasticsearch.common.text.Text; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.mapper.MappedFieldType; +import org.elasticsearch.lucene.search.uhighlight.QueryMaxAnalyzedOffset; import org.elasticsearch.search.fetch.FetchContext; import org.elasticsearch.search.fetch.FetchSubPhase; @@ -107,7 +108,10 @@ public HighlightField highlight(FieldHighlightContext fieldContext) throws IOExc ArrayList fragsList = new ArrayList<>(); List textsToHighlight; final int maxAnalyzedOffset = context.getSearchExecutionContext().getIndexSettings().getHighlightMaxAnalyzedOffset(); - Integer queryMaxAnalyzedOffset = fieldContext.field.fieldOptions().maxAnalyzedOffset(); + QueryMaxAnalyzedOffset queryMaxAnalyzedOffset = QueryMaxAnalyzedOffset.create( + fieldContext.field.fieldOptions().maxAnalyzedOffset(), + maxAnalyzedOffset + ); Analyzer analyzer = wrapAnalyzer( context.getSearchExecutionContext().getIndexAnalyzer(f -> Lucene.KEYWORD_ANALYZER), queryMaxAnalyzedOffset @@ -119,7 +123,8 @@ public HighlightField highlight(FieldHighlightContext fieldContext) throws IOExc for (Object textToHighlight : textsToHighlight) { String text = convertFieldValue(fieldType, textToHighlight); int textLength = text.length(); - if ((queryMaxAnalyzedOffset == null || queryMaxAnalyzedOffset > maxAnalyzedOffset) && (textLength > maxAnalyzedOffset)) { + if ((queryMaxAnalyzedOffset == null || queryMaxAnalyzedOffset.getNotNull() > maxAnalyzedOffset) + && (textLength > maxAnalyzedOffset)) { throw new IllegalArgumentException( "The length [" + textLength @@ -241,9 +246,9 @@ private static int findGoodEndForNoHighlightExcerpt(int noMatchSize, Analyzer an } } - private static Analyzer wrapAnalyzer(Analyzer analyzer, Integer maxAnalyzedOffset) { + private static Analyzer wrapAnalyzer(Analyzer analyzer, QueryMaxAnalyzedOffset maxAnalyzedOffset) { if (maxAnalyzedOffset != null) { - return new LimitTokenOffsetAnalyzer(analyzer, maxAnalyzedOffset); + return new LimitTokenOffsetAnalyzer(analyzer, maxAnalyzedOffset.getNotNull()); } return analyzer; } diff --git a/server/src/test/java/org/elasticsearch/lucene/search/uhighlight/CustomUnifiedHighlighterTests.java b/server/src/test/java/org/elasticsearch/lucene/search/uhighlight/CustomUnifiedHighlighterTests.java index 126641037fde7..2c400fe7b9173 100644 --- a/server/src/test/java/org/elasticsearch/lucene/search/uhighlight/CustomUnifiedHighlighterTests.java +++ b/server/src/test/java/org/elasticsearch/lucene/search/uhighlight/CustomUnifiedHighlighterTests.java @@ -157,7 +157,7 @@ private void assertHighlightOneDoc( noMatchSize, expectedPassages.length, maxAnalyzedOffset, - queryMaxAnalyzedOffset, + QueryMaxAnalyzedOffset.create(queryMaxAnalyzedOffset, maxAnalyzedOffset), true, true ); From f26f2c7ee815bb6d3f67b061c904eee8a49138aa Mon Sep 17 00:00:00 2001 From: Svilen Mihaylov Date: Thu, 2 Jan 2025 11:38:53 -0500 Subject: [PATCH 02/18] Add test --- .../highlight/HighlighterSearchIT.java | 36 +++++++++++++++++++ .../highlight/AbstractHighlighterBuilder.java | 6 +--- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/highlight/HighlighterSearchIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/highlight/HighlighterSearchIT.java index fc105d3d4fcd2..6f2b3da5279f8 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/highlight/HighlighterSearchIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/highlight/HighlighterSearchIT.java @@ -2632,6 +2632,42 @@ public void testPostingsHighlighterOrderByScore() throws Exception { }); } + public void testMaxQueryOffsetDefault() throws Exception { + assertAcked( + prepareCreate("test").setMapping(type1PostingsffsetsMapping()) + .setSettings(Settings.builder().put("index.highlight.max_analyzed_offset", "10").build()) + ); + ensureGreen(); + + prepareIndex("test").setSource( + "field1", + new String[] { + "This sentence contains one match, not that short. This sentence contains zero sentence matches. " + + "This one contains no matches.", + "This is the second value's first sentence. This one contains no matches. " + + "This sentence contains three sentence occurrences (sentence).", + "One sentence match here and scored lower since the text is quite long, not that appealing. " + + "This one contains no matches." } + ).get(); + refresh(); + + logger.info("--> highlighting and searching on field1"); + // Specific for this test: by passing "-1" as "maxAnalyzedOffset", the index highlight setting above will be used. + SearchSourceBuilder source = searchSource().query(termQuery("field1", "sentence")) + .highlighter(highlight().field("field1").order("score").maxAnalyzedOffset(-1)); + + assertResponse(client().search(new SearchRequest("test").source(source)), response -> { + Map highlightFieldMap = response.getHits().getAt(0).getHighlightFields(); + assertThat(highlightFieldMap.size(), equalTo(1)); + HighlightField field1 = highlightFieldMap.get("field1"); + assertThat(field1.fragments().length, equalTo(1)); + assertThat( + field1.fragments()[0].string(), + equalTo("This sentence contains one match, not that short. This sentence contains zero sentence matches.") + ); + }); + } + public void testPostingsHighlighterEscapeHtml() throws Exception { assertAcked(prepareCreate("test").setMapping("title", "type=text," + randomStoreField() + "index_options=offsets")); diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/AbstractHighlighterBuilder.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/AbstractHighlighterBuilder.java index a8db0f26d2966..f1f8961fd977b 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/AbstractHighlighterBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/AbstractHighlighterBuilder.java @@ -561,14 +561,10 @@ public Integer phraseLimit() { } /** - * Set to a non-negative value which represents the max offset used to analyze - * the field thus avoiding exceptions if the field exceeds this limit. + * "maxAnalyzedOffset" might be non-negative int, null (unknown), or a negative int (defaulting to index analyzed offset). */ @SuppressWarnings("unchecked") public HB maxAnalyzedOffset(Integer maxAnalyzedOffset) { - if (maxAnalyzedOffset != null && maxAnalyzedOffset <= 0) { - throw new IllegalArgumentException("[" + MAX_ANALYZED_OFFSET_FIELD + "] must be a positive integer"); - } this.maxAnalyzedOffset = maxAnalyzedOffset; return (HB) this; } From b18a696885617866ba1643a6f7f61e80c6a814e8 Mon Sep 17 00:00:00 2001 From: Svilen Mihaylov Date: Thu, 2 Jan 2025 12:18:48 -0500 Subject: [PATCH 03/18] Tweak tests --- .../test/search.highlight/30_max_analyzed_offset.yml | 4 ++-- .../fetch/subphase/highlight/AbstractHighlighterBuilder.java | 3 +++ .../fetch/subphase/highlight/HighlightBuilderTests.java | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml index d732fb084db3d..90a5765f86f27 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml @@ -115,7 +115,7 @@ setup: - match: {hits.hits.0.highlight.field2.0: "The quick brown fox went to the forest and saw another fox."} --- -"Plain highlighter with max_analyzed_offset < 0 should FAIL": +"Plain highlighter with max_analyzed_offset < -1 should FAIL": - requires: cluster_features: ["gte_v7.12.0"] @@ -130,4 +130,4 @@ setup: - match: { status: 400 } - match: { error.root_cause.0.type: "x_content_parse_exception" } - match: { error.caused_by.type: "illegal_argument_exception" } - - match: { error.caused_by.reason: "[max_analyzed_offset] must be a positive integer" } + - match: { error.caused_by.reason: "[max_analyzed_offset] must be an integer >= -1" } diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/AbstractHighlighterBuilder.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/AbstractHighlighterBuilder.java index f1f8961fd977b..3611c0e697826 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/AbstractHighlighterBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/AbstractHighlighterBuilder.java @@ -565,6 +565,9 @@ public Integer phraseLimit() { */ @SuppressWarnings("unchecked") public HB maxAnalyzedOffset(Integer maxAnalyzedOffset) { + if (maxAnalyzedOffset != null && maxAnalyzedOffset < -1) { + throw new IllegalArgumentException("[" + MAX_ANALYZED_OFFSET_FIELD + "] must be an integer >= -1"); + } this.maxAnalyzedOffset = maxAnalyzedOffset; return (HB) this; } diff --git a/server/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java b/server/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java index d1bbc1ec5910b..814a4d2d9c670 100644 --- a/server/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java @@ -580,7 +580,7 @@ public void testInvalidMaxAnalyzedOffset() throws IOException { "{ \"max_analyzed_offset\" : " + randomIntBetween(-100, 0) + "}" ); assertThat(e.getMessage(), containsString("[highlight] failed to parse field [" + MAX_ANALYZED_OFFSET_FIELD.toString() + "]")); - assertThat(e.getCause().getMessage(), containsString("[max_analyzed_offset] must be a positive integer")); + assertThat(e.getCause().getMessage(), containsString("[max_analyzed_offset] must be an integer >= -1")); } /** From f139e85af2513059a850a0f10234e51f11325f32 Mon Sep 17 00:00:00 2001 From: Svilen Mihaylov Date: Thu, 2 Jan 2025 16:57:55 -0500 Subject: [PATCH 04/18] Fix test and update docs --- .../search/search-your-data/highlighting.asciidoc | 6 ++++-- .../search.highlight/30_max_analyzed_offset.yml | 14 ++++++++++++++ .../subphase/highlight/HighlightBuilderTests.java | 2 +- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/docs/reference/search/search-your-data/highlighting.asciidoc b/docs/reference/search/search-your-data/highlighting.asciidoc index 6a432e6104524..7df46340073c9 100644 --- a/docs/reference/search/search-your-data/highlighting.asciidoc +++ b/docs/reference/search/search-your-data/highlighting.asciidoc @@ -262,9 +262,11 @@ max_analyzed_offset:: By default, the maximum number of characters analyzed for a highlight request is bounded by the value defined in the <> setting, and when the number of characters exceeds this limit an error is returned. If -this setting is set to a non-negative value, the highlighting stops at this defined +this setting is set to a non-negative (>=0) value, the highlighting stops at this defined maximum limit, and the rest of the text is not processed, thus not highlighted and -no error is returned. The <> query setting +no error is returned. If it is set to -1 then the value of +<> is used instead. +For values <= -1, an error is returned. The <> query setting does *not* override the <> which prevails when it's set to lower value than the query setting. diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml index 90a5765f86f27..030910c106b13 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml @@ -114,6 +114,20 @@ setup: body: {"query" : {"match" : {"field2" : "fox"}}, "highlight" : {"type" : "plain", "fields" : {"field2" : {}}, "max_analyzed_offset": 20}} - match: {hits.hits.0.highlight.field2.0: "The quick brown fox went to the forest and saw another fox."} +--- +"Plain highlighter on a field WITH OFFSETS exceeding index.highlight.max_analyzed_offset with max_analyzed_offset=1 should SUCCEED": + + - requires: + cluster_features: ["gte_v7.12.0"] + reason: max_analyzed_offset query param added in 7.12.0 + + - do: + search: + rest_total_hits_as_int: true + index: test1 + body: {"query" : {"match" : {"field2" : "fox"}}, "highlight" : {"type" : "plain", "fields" : {"field2" : {}}, "max_analyzed_offset": 1}} + - match: { hits.hits.0.highlight: null } + --- "Plain highlighter with max_analyzed_offset < -1 should FAIL": diff --git a/server/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java b/server/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java index 814a4d2d9c670..e0a60a48a876f 100644 --- a/server/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java @@ -577,7 +577,7 @@ public void testPreTagsWithoutPostTags() throws IOException { public void testInvalidMaxAnalyzedOffset() throws IOException { XContentParseException e = expectParseThrows( XContentParseException.class, - "{ \"max_analyzed_offset\" : " + randomIntBetween(-100, 0) + "}" + "{ \"max_analyzed_offset\" : " + randomIntBetween(-100, -1) + "}" ); assertThat(e.getMessage(), containsString("[highlight] failed to parse field [" + MAX_ANALYZED_OFFSET_FIELD.toString() + "]")); assertThat(e.getCause().getMessage(), containsString("[max_analyzed_offset] must be an integer >= -1")); From 1285b0149c40a75129c583ee92837fde3572d535 Mon Sep 17 00:00:00 2001 From: Svilen Mihaylov Date: Thu, 2 Jan 2025 17:02:36 -0500 Subject: [PATCH 05/18] Remove logger info --- .../search/fetch/subphase/highlight/HighlighterSearchIT.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/highlight/HighlighterSearchIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/highlight/HighlighterSearchIT.java index 6f2b3da5279f8..ee04ae624b16f 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/highlight/HighlighterSearchIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/highlight/HighlighterSearchIT.java @@ -2651,7 +2651,6 @@ public void testMaxQueryOffsetDefault() throws Exception { ).get(); refresh(); - logger.info("--> highlighting and searching on field1"); // Specific for this test: by passing "-1" as "maxAnalyzedOffset", the index highlight setting above will be used. SearchSourceBuilder source = searchSource().query(termQuery("field1", "sentence")) .highlighter(highlight().field("field1").order("score").maxAnalyzedOffset(-1)); From eb6725e0663f435c3b770889fde06f99e397d5b9 Mon Sep 17 00:00:00 2001 From: Svilen Mihaylov Date: Thu, 2 Jan 2025 17:39:05 -0500 Subject: [PATCH 06/18] Added test for zero --- .../search.highlight/30_max_analyzed_offset.yml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml index 030910c106b13..a7efec124cb3b 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml @@ -114,6 +114,20 @@ setup: body: {"query" : {"match" : {"field2" : "fox"}}, "highlight" : {"type" : "plain", "fields" : {"field2" : {}}, "max_analyzed_offset": 20}} - match: {hits.hits.0.highlight.field2.0: "The quick brown fox went to the forest and saw another fox."} +--- +"Plain highlighter on a field WITH OFFSETS exceeding index.highlight.max_analyzed_offset with max_analyzed_offset=0 should SUCCEED": + + - requires: + cluster_features: ["gte_v7.12.0"] + reason: max_analyzed_offset query param added in 7.12.0 + + - do: + search: + rest_total_hits_as_int: true + index: test1 + body: {"query" : {"match" : {"field2" : "fox"}}, "highlight" : {"type" : "plain", "fields" : {"field2" : {}}, "max_analyzed_offset": 0}} + - match: { hits.hits.0.highlight: null } + --- "Plain highlighter on a field WITH OFFSETS exceeding index.highlight.max_analyzed_offset with max_analyzed_offset=1 should SUCCEED": From 4ae0be1841b5e729afa4878c111f7ae35a5d17f9 Mon Sep 17 00:00:00 2001 From: Svilen Mihaylov Date: Fri, 3 Jan 2025 11:50:40 -0500 Subject: [PATCH 07/18] Add a capability --- .../test/search.highlight/30_max_analyzed_offset.yml | 8 ++++++-- .../rest/action/search/SearchCapabilities.java | 3 +++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml index a7efec124cb3b..7c68191b597ff 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml @@ -146,8 +146,12 @@ setup: "Plain highlighter with max_analyzed_offset < -1 should FAIL": - requires: - cluster_features: ["gte_v7.12.0"] - reason: max_analyzed_offset query param added in 7.12.0 + test_runner_features: [capabilities] + capabilities: + - method: GET + path: /_search + capabilities: [ max_analyzed_offset_default ] + reason: Behavior of max_analyzed_offset query param changed in 8.18. - do: catch: bad_request diff --git a/server/src/main/java/org/elasticsearch/rest/action/search/SearchCapabilities.java b/server/src/main/java/org/elasticsearch/rest/action/search/SearchCapabilities.java index 06f8f8f3c1be6..3b8242ae07419 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/search/SearchCapabilities.java +++ b/server/src/main/java/org/elasticsearch/rest/action/search/SearchCapabilities.java @@ -49,6 +49,8 @@ private SearchCapabilities() {} private static final String OPTIMIZED_SCALAR_QUANTIZATION_BBQ = "optimized_scalar_quantization_bbq"; private static final String KNN_QUANTIZED_VECTOR_RESCORE = "knn_quantized_vector_rescore"; + private static final String MAX_ANALYZED_OFFSET_DEFAULT = "max_analyzed_offset_default"; + public static final Set CAPABILITIES; static { HashSet capabilities = new HashSet<>(); @@ -70,6 +72,7 @@ private SearchCapabilities() {} if (Build.current().isSnapshot()) { capabilities.add(KQL_QUERY_SUPPORTED); } + capabilities.add(MAX_ANALYZED_OFFSET_DEFAULT); CAPABILITIES = Set.copyOf(capabilities); } } From c0c2127af1552d467e88454c153e15e08a333cff Mon Sep 17 00:00:00 2001 From: Svilen Mihaylov Date: Fri, 3 Jan 2025 12:40:33 -0500 Subject: [PATCH 08/18] Add test for -1 --- .../30_max_analyzed_offset.yml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml index 7c68191b597ff..4b7094cc28c2a 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml @@ -142,6 +142,24 @@ setup: body: {"query" : {"match" : {"field2" : "fox"}}, "highlight" : {"type" : "plain", "fields" : {"field2" : {}}, "max_analyzed_offset": 1}} - match: { hits.hits.0.highlight: null } +--- +"Plain highlighter with max_analyzed_offset = -1 should default to index analyze offset": + + - requires: + test_runner_features: [capabilities] + capabilities: + - method: GET + path: /_search + capabilities: [ max_analyzed_offset_default ] + reason: Behavior of max_analyzed_offset query param changed in 8.18. + + - do: + search: + rest_total_hits_as_int: true + index: test1 + body: {"query" : {"match" : {"field2" : "fox"}}, "highlight" : {"type" : "plain", "fields" : {"field2" : {}}, "max_analyzed_offset": -1}} + - match: {hits.hits.0.highlight.field2.0: "The quick brown fox went to the forest and saw another fox."} + --- "Plain highlighter with max_analyzed_offset < -1 should FAIL": From c6d2920b1e6657a145a69f3a9d2cd602ce8a0d39 Mon Sep 17 00:00:00 2001 From: Svilen Mihaylov Date: Fri, 3 Jan 2025 14:43:38 -0500 Subject: [PATCH 09/18] Exclude test from REST compatibility --- rest-api-spec/build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/rest-api-spec/build.gradle b/rest-api-spec/build.gradle index 147b04ccd6722..cb773c8ab311d 100644 --- a/rest-api-spec/build.gradle +++ b/rest-api-spec/build.gradle @@ -76,6 +76,7 @@ tasks.named("yamlRestCompatTestTransform").configure ({ task -> task.skipTest("logsdb/20_source_mapping/include/exclude is supported with stored _source", "no longer serialize source_mode") task.skipTest("logsdb/20_source_mapping/synthetic _source is default", "no longer serialize source_mode") task.skipTest("search/520_fetch_fields/fetch _seq_no via fields", "error code is changed from 5xx to 400 in 9.0") + task.skipTest("search.highlight/30_max_analyzed_offset/Plain highlighter with max_analyzed_offset < 0 should FAIL", "semantics of test has changed") task.skipTest("search.vectors/41_knn_search_bbq_hnsw/Test knn search", "Scoring has changed in latest versions") task.skipTest("search.vectors/42_knn_search_bbq_flat/Test knn search", "Scoring has changed in latest versions") task.skipTest("synonyms/90_synonyms_reloading_for_synset/Reload analyzers for specific synonym set", "Can't work until auto-expand replicas is 0-1 for synonyms index") From 703f4380012282ad6568c95783dc995ce42dc25c Mon Sep 17 00:00:00 2001 From: Svilen Mihaylov Date: Fri, 3 Jan 2025 17:57:01 -0500 Subject: [PATCH 10/18] Add Highlight_ to search capability name --- .../test/search.highlight/30_max_analyzed_offset.yml | 4 ++-- .../elasticsearch/rest/action/search/SearchCapabilities.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml index 4b7094cc28c2a..ee7708c775682 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml @@ -150,7 +150,7 @@ setup: capabilities: - method: GET path: /_search - capabilities: [ max_analyzed_offset_default ] + capabilities: [ highlight_max_analyzed_offset_default ] reason: Behavior of max_analyzed_offset query param changed in 8.18. - do: @@ -168,7 +168,7 @@ setup: capabilities: - method: GET path: /_search - capabilities: [ max_analyzed_offset_default ] + capabilities: [ highlight_max_analyzed_offset_default ] reason: Behavior of max_analyzed_offset query param changed in 8.18. - do: diff --git a/server/src/main/java/org/elasticsearch/rest/action/search/SearchCapabilities.java b/server/src/main/java/org/elasticsearch/rest/action/search/SearchCapabilities.java index 3b8242ae07419..872d0ec329765 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/search/SearchCapabilities.java +++ b/server/src/main/java/org/elasticsearch/rest/action/search/SearchCapabilities.java @@ -49,7 +49,7 @@ private SearchCapabilities() {} private static final String OPTIMIZED_SCALAR_QUANTIZATION_BBQ = "optimized_scalar_quantization_bbq"; private static final String KNN_QUANTIZED_VECTOR_RESCORE = "knn_quantized_vector_rescore"; - private static final String MAX_ANALYZED_OFFSET_DEFAULT = "max_analyzed_offset_default"; + private static final String HIGHLIGHT_MAX_ANALYZED_OFFSET_DEFAULT = "highlight_max_analyzed_offset_default"; public static final Set CAPABILITIES; static { @@ -72,7 +72,7 @@ private SearchCapabilities() {} if (Build.current().isSnapshot()) { capabilities.add(KQL_QUERY_SUPPORTED); } - capabilities.add(MAX_ANALYZED_OFFSET_DEFAULT); + capabilities.add(HIGHLIGHT_MAX_ANALYZED_OFFSET_DEFAULT); CAPABILITIES = Set.copyOf(capabilities); } } From 9fc0fcba296700cc310eb3d3d74e7f68ffb28d92 Mon Sep 17 00:00:00 2001 From: Svilen Mihaylov Date: Mon, 6 Jan 2025 11:22:43 -0500 Subject: [PATCH 11/18] Disallow 0 --- .../search/search-your-data/highlighting.asciidoc | 6 +++--- .../test/search.highlight/30_max_analyzed_offset.yml | 10 +++++++--- .../subphase/highlight/AbstractHighlighterBuilder.java | 4 ++-- .../subphase/highlight/HighlightBuilderTests.java | 2 +- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/docs/reference/search/search-your-data/highlighting.asciidoc b/docs/reference/search/search-your-data/highlighting.asciidoc index 7df46340073c9..63d9c632bffcf 100644 --- a/docs/reference/search/search-your-data/highlighting.asciidoc +++ b/docs/reference/search/search-your-data/highlighting.asciidoc @@ -262,11 +262,11 @@ max_analyzed_offset:: By default, the maximum number of characters analyzed for a highlight request is bounded by the value defined in the <> setting, and when the number of characters exceeds this limit an error is returned. If -this setting is set to a non-negative (>=0) value, the highlighting stops at this defined +this setting is set to a positive value, the highlighting stops at this defined maximum limit, and the rest of the text is not processed, thus not highlighted and -no error is returned. If it is set to -1 then the value of +no error is returned. If it is specifically set to -1 then the value of <> is used instead. -For values <= -1, an error is returned. The <> query setting +For values < -1 or 0, an error is returned. The <> query setting does *not* override the <> which prevails when it's set to lower value than the query setting. diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml index ee7708c775682..bf02fa8f3a81e 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml @@ -118,8 +118,12 @@ setup: "Plain highlighter on a field WITH OFFSETS exceeding index.highlight.max_analyzed_offset with max_analyzed_offset=0 should SUCCEED": - requires: - cluster_features: ["gte_v7.12.0"] - reason: max_analyzed_offset query param added in 7.12.0 + test_runner_features: [capabilities] + capabilities: + - method: GET + path: /_search + capabilities: [ highlight_max_analyzed_offset_default ] + reason: Behavior of max_analyzed_offset query param changed in 8.18. - do: search: @@ -180,4 +184,4 @@ setup: - match: { status: 400 } - match: { error.root_cause.0.type: "x_content_parse_exception" } - match: { error.caused_by.type: "illegal_argument_exception" } - - match: { error.caused_by.reason: "[max_analyzed_offset] must be an integer >= -1" } + - match: { error.caused_by.reason: "[max_analyzed_offset] must be a positive integer, or -1" } diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/AbstractHighlighterBuilder.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/AbstractHighlighterBuilder.java index 3611c0e697826..64b1d76f05122 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/AbstractHighlighterBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/AbstractHighlighterBuilder.java @@ -565,8 +565,8 @@ public Integer phraseLimit() { */ @SuppressWarnings("unchecked") public HB maxAnalyzedOffset(Integer maxAnalyzedOffset) { - if (maxAnalyzedOffset != null && maxAnalyzedOffset < -1) { - throw new IllegalArgumentException("[" + MAX_ANALYZED_OFFSET_FIELD + "] must be an integer >= -1"); + if (maxAnalyzedOffset != null && (maxAnalyzedOffset < -1 || maxAnalyzedOffset == 0)) { + throw new IllegalArgumentException("[" + MAX_ANALYZED_OFFSET_FIELD + "] must be a positive integer, or -1"); } this.maxAnalyzedOffset = maxAnalyzedOffset; return (HB) this; diff --git a/server/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java b/server/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java index e0a60a48a876f..62098faddb618 100644 --- a/server/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java @@ -580,7 +580,7 @@ public void testInvalidMaxAnalyzedOffset() throws IOException { "{ \"max_analyzed_offset\" : " + randomIntBetween(-100, -1) + "}" ); assertThat(e.getMessage(), containsString("[highlight] failed to parse field [" + MAX_ANALYZED_OFFSET_FIELD.toString() + "]")); - assertThat(e.getCause().getMessage(), containsString("[max_analyzed_offset] must be an integer >= -1")); + assertThat(e.getCause().getMessage(), containsString("[max_analyzed_offset] must be a positive integer, or -1")); } /** From 6fd85f38b494e5a6487ade6b87644c85f815a728 Mon Sep 17 00:00:00 2001 From: Svilen Mihaylov Date: Mon, 6 Jan 2025 11:25:27 -0500 Subject: [PATCH 12/18] Fix test --- .../test/search.highlight/30_max_analyzed_offset.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml index bf02fa8f3a81e..fb0e9a845bb79 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml @@ -130,7 +130,10 @@ setup: rest_total_hits_as_int: true index: test1 body: {"query" : {"match" : {"field2" : "fox"}}, "highlight" : {"type" : "plain", "fields" : {"field2" : {}}, "max_analyzed_offset": 0}} - - match: { hits.hits.0.highlight: null } + - match: { status: 400 } + - match: { error.root_cause.0.type: "x_content_parse_exception" } + - match: { error.caused_by.type: "illegal_argument_exception" } + - match: { error.caused_by.reason: "[max_analyzed_offset] must be a positive integer, or -1" } --- "Plain highlighter on a field WITH OFFSETS exceeding index.highlight.max_analyzed_offset with max_analyzed_offset=1 should SUCCEED": From 5abd2c4303ca10e1da7631d19ae4eca9f18fad23 Mon Sep 17 00:00:00 2001 From: Svilen Mihaylov Date: Mon, 6 Jan 2025 12:33:41 -0500 Subject: [PATCH 13/18] Should SUCCEED -> Should FAIL --- .../test/search.highlight/30_max_analyzed_offset.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml index fb0e9a845bb79..a873281f2fbd6 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml @@ -136,7 +136,7 @@ setup: - match: { error.caused_by.reason: "[max_analyzed_offset] must be a positive integer, or -1" } --- -"Plain highlighter on a field WITH OFFSETS exceeding index.highlight.max_analyzed_offset with max_analyzed_offset=1 should SUCCEED": +"Plain highlighter on a field WITH OFFSETS exceeding index.highlight.max_analyzed_offset with max_analyzed_offset=1 should FAIL": - requires: cluster_features: ["gte_v7.12.0"] From 08674800875e576bcd6ee7f497ec24e4039ebc3d Mon Sep 17 00:00:00 2001 From: Svilen Mihaylov Date: Mon, 6 Jan 2025 12:34:52 -0500 Subject: [PATCH 14/18] Oops wrong test --- .../test/search.highlight/30_max_analyzed_offset.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml index a873281f2fbd6..b97b052da3bd6 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml @@ -115,7 +115,7 @@ setup: - match: {hits.hits.0.highlight.field2.0: "The quick brown fox went to the forest and saw another fox."} --- -"Plain highlighter on a field WITH OFFSETS exceeding index.highlight.max_analyzed_offset with max_analyzed_offset=0 should SUCCEED": +"Plain highlighter on a field WITH OFFSETS exceeding index.highlight.max_analyzed_offset with max_analyzed_offset=0 should FAIL": - requires: test_runner_features: [capabilities] @@ -136,7 +136,7 @@ setup: - match: { error.caused_by.reason: "[max_analyzed_offset] must be a positive integer, or -1" } --- -"Plain highlighter on a field WITH OFFSETS exceeding index.highlight.max_analyzed_offset with max_analyzed_offset=1 should FAIL": +"Plain highlighter on a field WITH OFFSETS exceeding index.highlight.max_analyzed_offset with max_analyzed_offset=1 should SUCCEED": - requires: cluster_features: ["gte_v7.12.0"] From f889a61c5c211582d67e7b304f8b1303babe2521 Mon Sep 17 00:00:00 2001 From: Svilen Mihaylov Date: Mon, 6 Jan 2025 12:36:40 -0500 Subject: [PATCH 15/18] Rename another test --- .../test/search.highlight/30_max_analyzed_offset.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml index b97b052da3bd6..60dde5b068ecc 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml @@ -150,7 +150,7 @@ setup: - match: { hits.hits.0.highlight: null } --- -"Plain highlighter with max_analyzed_offset = -1 should default to index analyze offset": +"Plain highlighter with max_analyzed_offset = -1 default to index analyze offset should SUCCEED": - requires: test_runner_features: [capabilities] From dc54b64c186a47fd75adedfdb28a6b65b40270ef Mon Sep 17 00:00:00 2001 From: Svilen Mihaylov Date: Mon, 6 Jan 2025 13:03:09 -0500 Subject: [PATCH 16/18] Remove test with =0 for now. --- .../30_max_analyzed_offset.yml | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml index 60dde5b068ecc..ec855a6e557d4 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml @@ -114,27 +114,6 @@ setup: body: {"query" : {"match" : {"field2" : "fox"}}, "highlight" : {"type" : "plain", "fields" : {"field2" : {}}, "max_analyzed_offset": 20}} - match: {hits.hits.0.highlight.field2.0: "The quick brown fox went to the forest and saw another fox."} ---- -"Plain highlighter on a field WITH OFFSETS exceeding index.highlight.max_analyzed_offset with max_analyzed_offset=0 should FAIL": - - - requires: - test_runner_features: [capabilities] - capabilities: - - method: GET - path: /_search - capabilities: [ highlight_max_analyzed_offset_default ] - reason: Behavior of max_analyzed_offset query param changed in 8.18. - - - do: - search: - rest_total_hits_as_int: true - index: test1 - body: {"query" : {"match" : {"field2" : "fox"}}, "highlight" : {"type" : "plain", "fields" : {"field2" : {}}, "max_analyzed_offset": 0}} - - match: { status: 400 } - - match: { error.root_cause.0.type: "x_content_parse_exception" } - - match: { error.caused_by.type: "illegal_argument_exception" } - - match: { error.caused_by.reason: "[max_analyzed_offset] must be a positive integer, or -1" } - --- "Plain highlighter on a field WITH OFFSETS exceeding index.highlight.max_analyzed_offset with max_analyzed_offset=1 should SUCCEED": From 909d90fecac784692afd76862e0e98ef06f9dd73 Mon Sep 17 00:00:00 2001 From: Svilen Mihaylov Date: Mon, 6 Jan 2025 14:38:53 -0500 Subject: [PATCH 17/18] Restore test with zero --- .../30_max_analyzed_offset.yml | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml index ec855a6e557d4..9b3291e19c5fd 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/30_max_analyzed_offset.yml @@ -114,6 +114,28 @@ setup: body: {"query" : {"match" : {"field2" : "fox"}}, "highlight" : {"type" : "plain", "fields" : {"field2" : {}}, "max_analyzed_offset": 20}} - match: {hits.hits.0.highlight.field2.0: "The quick brown fox went to the forest and saw another fox."} +--- +"Plain highlighter on a field WITH OFFSETS exceeding index.highlight.max_analyzed_offset with max_analyzed_offset=0 should FAIL": + + - requires: + test_runner_features: [capabilities] + capabilities: + - method: GET + path: /_search + capabilities: [ highlight_max_analyzed_offset_default ] + reason: Behavior of max_analyzed_offset query param changed in 8.18. + + - do: + catch: bad_request + search: + rest_total_hits_as_int: true + index: test1 + body: {"query" : {"match" : {"field2" : "fox"}}, "highlight" : {"type" : "plain", "fields" : {"field2" : {}}, "max_analyzed_offset": 0}} + - match: { status: 400 } + - match: { error.root_cause.0.type: "x_content_parse_exception" } + - match: { error.caused_by.type: "illegal_argument_exception" } + - match: { error.caused_by.reason: "[max_analyzed_offset] must be a positive integer, or -1" } + --- "Plain highlighter on a field WITH OFFSETS exceeding index.highlight.max_analyzed_offset with max_analyzed_offset=1 should SUCCEED": From 15bc509a17f28242af8e1bd8e669d309366d4990 Mon Sep 17 00:00:00 2001 From: Svilen Mihaylov Date: Tue, 7 Jan 2025 10:11:26 -0500 Subject: [PATCH 18/18] Fix merge --- rest-api-spec/build.gradle | 1 - 1 file changed, 1 deletion(-) diff --git a/rest-api-spec/build.gradle b/rest-api-spec/build.gradle index 17994c03f137a..ed8a1f147b4fa 100644 --- a/rest-api-spec/build.gradle +++ b/rest-api-spec/build.gradle @@ -60,6 +60,5 @@ tasks.named("yamlRestCompatTestTransform").configure ({ task -> task.skipTest("cat.aliases/10_basic/Deprecated local parameter", "CAT APIs not covered by compatibility policy") task.skipTest("cat.shards/10_basic/Help", "sync_id is removed in 9.0") task.skipTest("search/500_date_range/from, to, include_lower, include_upper deprecated", "deprecated parameters are removed in 9.0") - task.skipTest("search/520_fetch_fields/fetch _seq_no via fields", "error code is changed from 5xx to 400 in 9.0") task.skipTest("search.highlight/30_max_analyzed_offset/Plain highlighter with max_analyzed_offset < 0 should FAIL", "semantics of test has changed") })