Merge pull request #19877 from areek/fix/remove_completion_payload

Remove payload option from completion suggester

Author: areek

core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggester.java

@@ -78,39 +78,16 @@ protected Suggest.Suggestion<? extends Suggest.Suggestion.Entry<? extends Sugges
             TopSuggestDocsCollector collector = new TopDocumentsCollector(suggestionContext.getSize());
             suggest(searcher, suggestionContext.toQuery(), collector);
             int numResult = 0;
-            List<LeafReaderContext> leaves = searcher.getIndexReader().leaves();
             for (TopSuggestDocs.SuggestScoreDoc suggestScoreDoc : collector.get().scoreLookupDocs()) {
                 TopDocumentsCollector.SuggestDoc suggestDoc = (TopDocumentsCollector.SuggestDoc) suggestScoreDoc;
                 // collect contexts
                 Map<String, Set<CharSequence>> contexts = Collections.emptyMap();
                 if (fieldType.hasContextMappings() && suggestDoc.getContexts().isEmpty() == false) {
                     contexts = fieldType.getContextMappings().getNamedContexts(suggestDoc.getContexts());
                 }
-                // collect payloads
-                final Map<String, List<Object>> payload = new HashMap<>(0);
-                List<String> payloadFields = suggestionContext.getPayloadFields();
-                if (payloadFields.isEmpty() == false) {
-                    final int readerIndex = ReaderUtil.subIndex(suggestDoc.doc, leaves);
-                    final LeafReaderContext subReaderContext = leaves.get(readerIndex);
-                    final int subDocId = suggestDoc.doc - subReaderContext.docBase;
-                    for (String field : payloadFields) {
-                        MapperService mapperService = suggestionContext.getShardContext().getMapperService();
-                        MappedFieldType payloadFieldType = mapperService.fullName(field);
-                        if (payloadFieldType != null) {
-                            QueryShardContext shardContext = suggestionContext.getShardContext();
-                            final AtomicFieldData data = shardContext.getForField(payloadFieldType)
-                                .load(subReaderContext);
-                            final ScriptDocValues scriptValues = data.getScriptValues();
-                            scriptValues.setNextDocId(subDocId);
-                            payload.put(field, new ArrayList<>(scriptValues.getValues()));
-                        } else {
-                            throw new IllegalArgumentException("payload field [" + field + "] does not exist");
-                        }
-                    }
-                }
                 if (numResult++ < suggestionContext.getSize()) {
                     CompletionSuggestion.Entry.Option option = new CompletionSuggestion.Entry.Option(suggestDoc.doc,
-                        new Text(suggestDoc.key.toString()), suggestDoc.score, contexts, payload);
+                        new Text(suggestDoc.key.toString()), suggestDoc.score, contexts);
                     completionSuggestEntry.addOption(option);
                 } else {
                     break;

core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestion.java

@@ -194,14 +194,12 @@ protected Option newOption() {
 
         public static class Option extends Suggest.Suggestion.Entry.Option {
             private Map<String, Set<CharSequence>> contexts;
-            private Map<String, List<Object>> payload;
             private ScoreDoc doc;
             private InternalSearchHit hit;
 
-            public Option(int docID, Text text, float score, Map<String, Set<CharSequence>> contexts, Map<String, List<Object>> payload) {
+            public Option(int docID, Text text, float score, Map<String, Set<CharSequence>> contexts) {
                 super(text, score);
                 this.doc = new ScoreDoc(docID, score);
-                this.payload = payload;
                 this.contexts = contexts;
             }
 
@@ -216,10 +214,6 @@ protected void mergeInto(Suggest.Suggestion.Entry.Option otherOption) {
                 throw new UnsupportedOperationException();
             }
 
-            public Map<String, List<Object>> getPayload() {
-                return payload;
-            }
-
             public Map<String, Set<CharSequence>> getContexts() {
                 return contexts;
             }
@@ -248,17 +242,6 @@ protected XContentBuilder innerToXContent(XContentBuilder builder, Params params
                 } else {
                     builder.field("score", getScore());
                 }
-                if (payload.size() > 0) {
-                    builder.startObject("payload");
-                    for (Map.Entry<String, List<Object>> entry : payload.entrySet()) {
-                        builder.startArray(entry.getKey());
-                        for (Object payload : entry.getValue()) {
-                            builder.value(payload);
-                        }
-                        builder.endArray();
-                    }
-                    builder.endObject();
-                }
                 if (contexts.size() > 0) {
                     builder.startObject("contexts");
                     for (Map.Entry<String, Set<CharSequence>> entry : contexts.entrySet()) {
@@ -281,17 +264,6 @@ public void readFrom(StreamInput in) throws IOException {
                     this.hit = InternalSearchHit.readSearchHit(in,
                         InternalSearchHits.streamContext().streamShardTarget(ShardTargetType.STREAM));
                 }
-                int payloadSize = in.readInt();
-                this.payload = new LinkedHashMap<>(payloadSize);
-                for (int i = 0; i < payloadSize; i++) {
-                    String payloadName = in.readString();
-                    int nValues = in.readVInt();
-                    List<Object> values = new ArrayList<>(nValues);
-                    for (int j = 0; j < nValues; j++) {
-                        values.add(in.readGenericValue());
-                    }
-                    this.payload.put(payloadName, values);
-                }
                 int contextSize = in.readInt();
                 this.contexts = new LinkedHashMap<>(contextSize);
                 for (int i = 0; i < contextSize; i++) {
@@ -315,15 +287,6 @@ public void writeTo(StreamOutput out) throws IOException {
                 } else {
                     out.writeBoolean(false);
                 }
-                out.writeInt(payload.size());
-                for (Map.Entry<String, List<Object>> entry : payload.entrySet()) {
-                    out.writeString(entry.getKey());
-                    List<Object> values = entry.getValue();
-                    out.writeVInt(values.size());
-                    for (Object value : values) {
-                        out.writeGenericValue(value);
-                    }
-                }
                 out.writeInt(contexts.size());
                 for (Map.Entry<String, Set<CharSequence>> entry : contexts.entrySet()) {
                     out.writeString(entry.getKey());
@@ -341,14 +304,6 @@ public String toString() {
                 stringBuilder.append(getText());
                 stringBuilder.append(" score:");
                 stringBuilder.append(getScore());
-                stringBuilder.append(" payload:[");
-                for (Map.Entry<String, List<Object>> entry : payload.entrySet()) {
-                    stringBuilder.append(" ");
-                    stringBuilder.append(entry.getKey());
-                    stringBuilder.append(":");
-                    stringBuilder.append(entry.getValue());
-                }
-                stringBuilder.append("]");
                 stringBuilder.append(" context:[");
                 for (Map.Entry<String, Set<CharSequence>> entry: contexts.entrySet()) {
                     stringBuilder.append(" ");

core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionBuilder.java

@@ -62,7 +62,6 @@
  */
 public class CompletionSuggestionBuilder extends SuggestionBuilder<CompletionSuggestionBuilder> {
     static final String SUGGESTION_NAME = "completion";
-    static final ParseField PAYLOAD_FIELD = new ParseField("payload");
     static final ParseField CONTEXTS_FIELD = new ParseField("contexts", "context");
 
     /**
@@ -78,7 +77,6 @@ public class CompletionSuggestionBuilder extends SuggestionBuilder<CompletionSug
     private static ObjectParser<CompletionSuggestionBuilder.InnerBuilder, ParseFieldMatcherSupplier> TLP_PARSER =
         new ObjectParser<>(SUGGESTION_NAME, null);
     static {
-        TLP_PARSER.declareStringArray(CompletionSuggestionBuilder.InnerBuilder::payload, PAYLOAD_FIELD);
         TLP_PARSER.declareField((parser, completionSuggestionContext, context) -> {
                 if (parser.currentToken() == XContentParser.Token.VALUE_BOOLEAN) {
                     if (parser.booleanValue()) {
@@ -108,7 +106,6 @@ public class CompletionSuggestionBuilder extends SuggestionBuilder<CompletionSug
     protected FuzzyOptions fuzzyOptions;
     protected RegexOptions regexOptions;
     protected BytesReference contextBytes = null;
-    protected List<String> payloadFields = Collections.emptyList();
 
     public CompletionSuggestionBuilder(String field) {
         super(field);
@@ -123,24 +120,20 @@ private CompletionSuggestionBuilder(String fieldname, CompletionSuggestionBuilde
         fuzzyOptions = in.fuzzyOptions;
         regexOptions = in.regexOptions;
         contextBytes = in.contextBytes;
-        payloadFields = in.payloadFields;
     }
 
     /**
      * Read from a stream.
      */
     public CompletionSuggestionBuilder(StreamInput in) throws IOException {
         super(in);
-        payloadFields = new ArrayList<>();
-        Collections.addAll(payloadFields, in.readStringArray());
         fuzzyOptions = in.readOptionalWriteable(FuzzyOptions::new);
         regexOptions = in.readOptionalWriteable(RegexOptions::new);
         contextBytes = in.readOptionalBytesReference();
     }
 
     @Override
     public void doWriteTo(StreamOutput out) throws IOException {
-        out.writeStringArray(payloadFields.toArray(new String[payloadFields.size()]));
         out.writeOptionalWriteable(fuzzyOptions);
         out.writeOptionalWriteable(regexOptions);
         out.writeOptionalBytesReference(contextBytes);
@@ -194,16 +187,6 @@ public CompletionSuggestionBuilder regex(String regex, RegexOptions regexOptions
         return this;
     }
 
-    /**
-     * Sets the fields to be returned as suggestion payload.
-     * Note: Only doc values enabled fields are supported
-     */
-    public CompletionSuggestionBuilder payload(List<String> fields) {
-        Objects.requireNonNull(fields, "payload must not be null");
-        this.payloadFields = fields;
-        return this;
-    }
-
     /**
      * Sets query contexts for completion
      * @param queryContexts named query contexts
@@ -348,13 +331,6 @@ private InnerBuilder field(String field) {
 
     @Override
     protected XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws IOException {
-        if (payloadFields.isEmpty() == false) {
-            builder.startArray(PAYLOAD_FIELD.getPreferredName());
-            for (String field : payloadFields) {
-                builder.value(field);
-            }
-            builder.endArray();
-        }
         if (fuzzyOptions != null) {
             fuzzyOptions.toXContent(builder, params);
         }
@@ -388,7 +364,6 @@ public SuggestionContext build(QueryShardContext context) throws IOException {
         // copy over common settings to each suggestion builder
         final MapperService mapperService = context.getMapperService();
         populateCommonFields(mapperService, suggestionContext);
-        suggestionContext.setPayloadFields(payloadFields);
         suggestionContext.setFuzzyOptions(fuzzyOptions);
         suggestionContext.setRegexOptions(regexOptions);
         MappedFieldType mappedFieldType = mapperService.fullName(suggestionContext.getField());
@@ -449,14 +424,13 @@ public String getWriteableName() {
 
     @Override
     protected boolean doEquals(CompletionSuggestionBuilder other) {
-        return Objects.equals(payloadFields, other.payloadFields) &&
-            Objects.equals(fuzzyOptions, other.fuzzyOptions) &&
+        return Objects.equals(fuzzyOptions, other.fuzzyOptions) &&
             Objects.equals(regexOptions, other.regexOptions) &&
             Objects.equals(contextBytes, other.contextBytes);
     }
 
     @Override
     protected int doHashCode() {
-        return Objects.hash(payloadFields, fuzzyOptions, regexOptions, contextBytes);
+        return Objects.hash(fuzzyOptions, regexOptions, contextBytes);
     }
 }

core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionContext.java

@@ -45,7 +45,6 @@ protected CompletionSuggestionContext(QueryShardContext shardContext) {
     private FuzzyOptions fuzzyOptions;
     private RegexOptions regexOptions;
     private Map<String, List<ContextMapping.InternalQueryContext>> queryContexts = Collections.emptyMap();
-    private List<String> payloadFields = Collections.emptyList();
     private CompletionFieldMapper2x.CompletionFieldType fieldType2x;
     private List<ContextQuery> contextQueries;
 
@@ -73,14 +72,6 @@ void setQueryContexts(Map<String, List<ContextMapping.InternalQueryContext>> que
         this.queryContexts = queryContexts;
     }
 
-    void setPayloadFields(List<String> fields) {
-        this.payloadFields = fields;
-    }
-
-    List<String> getPayloadFields() {
-        return payloadFields;
-    }
-
     public FuzzyOptions getFuzzyOptions() {
         return fuzzyOptions;
     }

core/src/test/java/org/elasticsearch/search/controller/SearchPhaseControllerTests.java

@@ -149,7 +149,7 @@ private AtomicArray<QuerySearchResultProvider> generateQueryResults(int nShards,
                 float maxScore = randomIntBetween(suggestion.getSize(), (int) Float.MAX_VALUE);
                 for (int i = 0; i < optionSize; i++) {
                     completionEntry.addOption(new CompletionSuggestion.Entry.Option(i, new Text(""), maxScore,
-                        Collections.emptyMap(), Collections.emptyMap()));
+                        Collections.emptyMap()));
                     float dec = randomIntBetween(0, optionSize);
                     if (dec <= maxScore) {
                         maxScore -= dec;