From 66b72a1a604df5067336a3b5f2becf4ae609c83f Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Wed, 29 Jan 2025 11:40:26 -0500 Subject: [PATCH 1/3] Support duplicate suggestions in completion field Currently if a document has duplicate suggestions across different contexts, only the first gets indexed, and when a user tries to search using the second context, she will get 0 results. This PR addresses this, but adding support for duplicate suggestions across different contexts, so documents like below with duplicate inputs can be searched across all provided contexts. ```json { "my_suggest": [ { "input": [ "foox", "boo" ], "weight" : 2, "contexts": { "color": [ "red" ] } }, { "input": [ "foox" ], "weight" : 3, "contexts": { "color": [ "blue" ] } } ] } ``` Closes #82432 --- .../rest-api-spec/test/suggest/30_context.yml | 72 ++++++++++++++ .../50_completion_with_multi_fields.yml | 77 +++++++++++++++ .../index/mapper/CompletionFieldMapper.java | 94 +++++++++++++++---- .../elasticsearch/search/SearchFeatures.java | 4 +- .../mapper/CompletionFieldMapperTests.java | 46 +++++++++ 5 files changed, 273 insertions(+), 20 deletions(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/suggest/30_context.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/suggest/30_context.yml index f88726469f51c..71b4ec9c128d8 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/suggest/30_context.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/suggest/30_context.yml @@ -395,3 +395,75 @@ setup: field: suggest_multi_contexts contexts: location: [] + +--- +"Duplicate suggestions in different contexts": + - requires: + cluster_features: [ "search.completion_field.duplicate.support" ] + reason: "Support for duplicate suggestions in different contexts" + + - do: + index: + refresh: true + index: test + id: "1" + body: + suggest_context: + - + input: "foox" + weight: 2 + contexts: + color: ["red", "yellow"] + - + input: "foox" + weight: 3 + contexts: + color: ["blue", "green", "yellow"] + - do: + search: + body: + suggest: + result: + text: "foo" + completion: + field: suggest_context + contexts: + color: "red" + + - length: { suggest.result: 1 } + - length: { suggest.result.0.options: 1 } + - match: { suggest.result.0.options.0.text: "foox" } + - match: { suggest.result.0.options.0._score: 2 } + + - do: + search: + body: + suggest: + result: + text: "foo" + completion: + field: suggest_context + contexts: + color: "yellow" + + - length: { suggest.result: 1 } + - length: { suggest.result.0.options: 1 } + - match: { suggest.result.0.options.0.text: "foox" } + # the highest weight wins + - match: { suggest.result.0.options.0._score: 3 } + + - do: + search: + body: + suggest: + result: + text: "foo" + completion: + field: suggest_context + contexts: + color: "blue" + + - length: { suggest.result: 1 } + - length: { suggest.result.0.options: 1 } + - match: { suggest.result.0.options.0.text: "foox" } + - match: { suggest.result.0.options.0._score: 3 } diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/suggest/50_completion_with_multi_fields.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/suggest/50_completion_with_multi_fields.yml index 8bbda56db7e53..37a937bd59b5a 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/suggest/50_completion_with_multi_fields.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/suggest/50_completion_with_multi_fields.yml @@ -268,3 +268,80 @@ - length: { suggest.result: 1 } - length: { suggest.result.0.options: 1 } + +--- +"Duplicate suggestions in different contexts in sub-fields": + - requires: + cluster_features: [ "search.completion_field.duplicate.support" ] + reason: "Support for duplicate suggestions in different contexts" + + - do: + indices.create: + index: completion_with_context + body: + mappings: + "properties": + "suggest_1": + "type": "completion" + "contexts": + - + "name": "color" + "type": "category" + "fields": + "suggest_2": + "type": "completion" + "contexts": + - + "name": "color" + "type": "category" + + + - do: + index: + refresh: true + index: completion_with_context + id: "1" + body: + suggest_1: + - + input: "foox" + weight: 2 + contexts: + color: ["red"] + - + input: "foox" + weight: 3 + contexts: + color: ["blue", "green"] + - do: + search: + body: + suggest: + result: + text: "foo" + completion: + field: suggest_1.suggest_2 + contexts: + color: "red" + + - length: { suggest.result: 1 } + - length: { suggest.result.0.options: 1 } + - match: { suggest.result.0.options.0.text: "foox" } + - match: { suggest.result.0.options.0._score: 2 } + + + - do: + search: + body: + suggest: + result: + text: "foo" + completion: + field: suggest_1.suggest_2 + contexts: + color: "blue" + + - length: { suggest.result: 1 } + - length: { suggest.result.0.options: 1 } + - match: { suggest.result.0.options.0.text: "foox" } + - match: { suggest.result.0.options.0._score: 3 } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java index f0c679d4f4994..b71d728d79bc1 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java @@ -392,7 +392,7 @@ public void parse(DocumentParserContext context) throws IOException { // parse XContentParser parser = context.parser(); Token token = parser.currentToken(); - Map inputMap = Maps.newMapWithExpectedSize(1); + Map inputMap = Maps.newMapWithExpectedSize(1); if (token == Token.VALUE_NULL) { // ignore null values return; @@ -405,7 +405,7 @@ public void parse(DocumentParserContext context) throws IOException { } // index - for (Map.Entry completionInput : inputMap.entrySet()) { + for (Map.Entry completionInput : inputMap.entrySet()) { String input = completionInput.getKey(); if (input.trim().isEmpty()) { context.addIgnoredField(mappedFieldType.name()); @@ -420,21 +420,33 @@ public void parse(DocumentParserContext context) throws IOException { } input = input.substring(0, len); } - CompletionInputMetadata metadata = completionInput.getValue(); + CompletionInputMetadataContainer cmc = completionInput.getValue(); if (fieldType().hasContextMappings()) { - fieldType().getContextMappings().addField(context.doc(), fieldType().name(), input, metadata.weight, metadata.contexts); + for (CompletionInputMetadata metadata : cmc.getValues()) { + fieldType().getContextMappings().addField(context.doc(), fieldType().name(), input, metadata.weight, metadata.contexts); + } } else { - context.doc().add(new SuggestField(fieldType().name(), input, metadata.weight)); + context.doc().add(new SuggestField(fieldType().name(), input, cmc.getWeight())); } } - context.addToFieldNames(fieldType().name()); - for (CompletionInputMetadata metadata : inputMap.values()) { - multiFields().parse( - this, - context, - () -> context.switchParser(new MultiFieldParser(metadata, fieldType().name(), context.parser().getTokenLocation())) - ); + for (CompletionInputMetadataContainer cmc : inputMap.values()) { + if (fieldType().hasContextMappings()) { + for (CompletionInputMetadata metadata : cmc.getValues()) { + multiFields().parse( + this, + context, + () -> context.switchParser(new MultiFieldParser(metadata, fieldType().name(), context.parser().getTokenLocation())) + ); + } + } else { + CompletionInputMetadata metadata = cmc.getValue(); + multiFields().parse( + this, + context, + () -> context.switchParser(new MultiFieldParser(metadata, fieldType().name(), context.parser().getTokenLocation())) + ); + } } } @@ -447,11 +459,13 @@ private void parse( DocumentParserContext documentParserContext, Token token, XContentParser parser, - Map inputMap + Map inputMap ) throws IOException { String currentFieldName = null; if (token == Token.VALUE_STRING) { - inputMap.put(parser.text(), new CompletionInputMetadata(parser.text(), Collections.>emptyMap(), 1)); + CompletionInputMetadataContainer cmc = new CompletionInputMetadataContainer(fieldType().hasContextMappings()); + cmc.add(new CompletionInputMetadata(parser.text(), Collections.emptyMap(), 1)); + inputMap.put(parser.text(), cmc); } else if (token == Token.START_OBJECT) { Set inputs = new HashSet<>(); int weight = 1; @@ -531,8 +545,14 @@ private void parse( } } for (String input : inputs) { - if (inputMap.containsKey(input) == false || inputMap.get(input).weight < weight) { - inputMap.put(input, new CompletionInputMetadata(input, contextsMap, weight)); + CompletionInputMetadata cm = new CompletionInputMetadata(input, contextsMap, weight); + CompletionInputMetadataContainer cmc = inputMap.get(input); + if (cmc != null) { + cmc.add(cm); + } else { + cmc = new CompletionInputMetadataContainer(fieldType().hasContextMappings()); + cmc.add(cm); + inputMap.put(input, cmc); } } } else { @@ -543,10 +563,46 @@ private void parse( } } + static class CompletionInputMetadataContainer { + private final boolean hasContexts; + private final List list; + private CompletionInputMetadata single; + + CompletionInputMetadataContainer(boolean hasContexts) { + this.hasContexts = hasContexts; + this.list = hasContexts ? new ArrayList<>() : null; + } + + void add(CompletionInputMetadata cm) { + if (hasContexts) { + list.add(cm); + } else { + if (single == null || single.weight < cm.weight) { + single = cm; + } + } + } + + List getValues() { + assert hasContexts; + return list; + } + + CompletionInputMetadata getValue() { + assert hasContexts == false; + return single; + } + + int getWeight() { + assert hasContexts == false; + return single.weight; + } + } + static class CompletionInputMetadata { - public final String input; - public final Map> contexts; - public final int weight; + private final String input; + private final Map> contexts; + private final int weight; CompletionInputMetadata(String input, Map> contexts, int weight) { this.input = input; diff --git a/server/src/main/java/org/elasticsearch/search/SearchFeatures.java b/server/src/main/java/org/elasticsearch/search/SearchFeatures.java index 8077da130c34e..8ea6f78ddfb41 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchFeatures.java +++ b/server/src/main/java/org/elasticsearch/search/SearchFeatures.java @@ -25,9 +25,11 @@ public Set getFeatures() { } public static final NodeFeature RETRIEVER_RESCORER_ENABLED = new NodeFeature("search.retriever.rescorer.enabled"); + public static final NodeFeature COMPLETION_FIELD_SUPPORTS_DUPLICATE_SUGGESTIONS = + new NodeFeature("search.completion_field.duplicate.support"); @Override public Set getTestFeatures() { - return Set.of(RETRIEVER_RESCORER_ENABLED); + return Set.of(RETRIEVER_RESCORER_ENABLED, COMPLETION_FIELD_SUPPORTS_DUPLICATE_SUGGESTIONS); } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java index 755d5dde2f162..7467ef6e38b30 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java @@ -303,6 +303,52 @@ public void testKeywordWithSubCompletionAndContext() throws Exception { ); } + + public void testDuplicateSuggestionsWithContexts() throws IOException { + DocumentMapper defaultMapper = createDocumentMapper(fieldMapping(b -> { + b.field("type", "completion"); + b.startArray("contexts"); + { + b.startObject(); + b.field("name", "place"); + b.field("type", "category"); + b.endObject(); + } + b.endArray(); + })); + + ParsedDocument parsedDocument = defaultMapper.parse(source(b -> { + b.startArray("field"); + { + b.startObject(); + { + b.array("input", "timmy", "starbucks"); + b.startObject("contexts").array("place", "cafe", "food").endObject(); + b.field("weight", 10); + } + b.endObject(); + b.startObject(); + { + b.array("input", "timmy", "starbucks"); + b.startObject("contexts").array("place", "restaurant").endObject(); + b.field("weight", 1); + } + b.endObject(); + } + b.endArray(); + })); + + List indexedFields = parsedDocument.rootDoc().getFields("field"); + assertThat(indexedFields, hasSize(4)); + + assertThat( + indexedFields, + containsInAnyOrder(contextSuggestField("timmy"), contextSuggestField("timmy"), + contextSuggestField("starbucks"), contextSuggestField("starbucks")) + ); + + } + public void testCompletionWithContextAndSubCompletion() throws Exception { DocumentMapper defaultMapper = createDocumentMapper(fieldMapping(b -> { b.field("type", "completion"); From 3d36b6f6a7d9ae26a73042c8bfd996eefd763289 Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Thu, 30 Jan 2025 14:03:07 -0500 Subject: [PATCH 2/3] Update docs/changelog/121324.yaml --- docs/changelog/121324.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/121324.yaml diff --git a/docs/changelog/121324.yaml b/docs/changelog/121324.yaml new file mode 100644 index 0000000000000..d105ea0b46b4c --- /dev/null +++ b/docs/changelog/121324.yaml @@ -0,0 +1,6 @@ +pr: 121324 +summary: Support duplicate suggestions in completion field +area: Suggesters +type: bug +issues: + - 82432 From 985d9d8365fda1db31f10d0801fbee3b7526b78f Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Thu, 30 Jan 2025 19:10:35 +0000 Subject: [PATCH 3/3] [CI] Auto commit changes from spotless --- .../index/mapper/CompletionFieldMapper.java | 4 ++-- .../java/org/elasticsearch/search/SearchFeatures.java | 5 +++-- .../index/mapper/CompletionFieldMapperTests.java | 9 ++++++--- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java index b71d728d79bc1..af691c61abe2e 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java @@ -563,7 +563,7 @@ private void parse( } } - static class CompletionInputMetadataContainer { + static class CompletionInputMetadataContainer { private final boolean hasContexts; private final List list; private CompletionInputMetadata single; @@ -573,7 +573,7 @@ static class CompletionInputMetadataContainer { this.list = hasContexts ? new ArrayList<>() : null; } - void add(CompletionInputMetadata cm) { + void add(CompletionInputMetadata cm) { if (hasContexts) { list.add(cm); } else { diff --git a/server/src/main/java/org/elasticsearch/search/SearchFeatures.java b/server/src/main/java/org/elasticsearch/search/SearchFeatures.java index 8ea6f78ddfb41..3970b6effe70c 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchFeatures.java +++ b/server/src/main/java/org/elasticsearch/search/SearchFeatures.java @@ -25,8 +25,9 @@ public Set getFeatures() { } public static final NodeFeature RETRIEVER_RESCORER_ENABLED = new NodeFeature("search.retriever.rescorer.enabled"); - public static final NodeFeature COMPLETION_FIELD_SUPPORTS_DUPLICATE_SUGGESTIONS = - new NodeFeature("search.completion_field.duplicate.support"); + public static final NodeFeature COMPLETION_FIELD_SUPPORTS_DUPLICATE_SUGGESTIONS = new NodeFeature( + "search.completion_field.duplicate.support" + ); @Override public Set getTestFeatures() { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java index 7467ef6e38b30..b093307f3733b 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java @@ -303,7 +303,6 @@ public void testKeywordWithSubCompletionAndContext() throws Exception { ); } - public void testDuplicateSuggestionsWithContexts() throws IOException { DocumentMapper defaultMapper = createDocumentMapper(fieldMapping(b -> { b.field("type", "completion"); @@ -343,8 +342,12 @@ public void testDuplicateSuggestionsWithContexts() throws IOException { assertThat( indexedFields, - containsInAnyOrder(contextSuggestField("timmy"), contextSuggestField("timmy"), - contextSuggestField("starbucks"), contextSuggestField("starbucks")) + containsInAnyOrder( + contextSuggestField("timmy"), + contextSuggestField("timmy"), + contextSuggestField("starbucks"), + contextSuggestField("starbucks") + ) ); }