Retrieve value from DocValues in a flat_object filed

Signed-off-by: kkewwei <kewei.11@bytedance.com> Signed-off-by: kkewwei <kkewwei@163.com>
opensearch-project · Dec 11, 2024 · c2e65ec · c2e65ec
1 parent 98dbc4a
commit c2e65ec
Show file tree

Hide file tree

Showing 7 changed files with 151 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Support prefix list for remote repository attributes([#16271](https://github.com/opensearch-project/OpenSearch/pull/16271))
 - Add new configuration setting `synonym_analyzer`, to the `synonym` and `synonym_graph` filters, enabling the specification of a custom analyzer for reading the synonym file ([#16488](https://github.com/opensearch-project/OpenSearch/pull/16488)).
 - Add stats for remote publication failure and move download failure stats to remote methods([#16682](https://github.com/opensearch-project/OpenSearch/pull/16682/))
+- Added ability to retrieve value from DocValues in a flat_object filed([#16802](https://github.com/opensearch-project/OpenSearch/pull/16802))
 
 ### Dependencies
 - Bump `com.google.cloud:google-cloud-core-http` from 2.23.0 to 2.47.0 ([#16504](https://github.com/opensearch-project/OpenSearch/pull/16504))

diff --git a/...pi-spec/src/main/resources/rest-api-spec/test/index/92_flat_object_support_doc_values.yml b/...pi-spec/src/main/resources/rest-api-spec/test/index/92_flat_object_support_doc_values.yml
@@ -1,8 +1,9 @@
 ---
 # The test setup includes:
-# - Create flat_object mapping for flat_object_doc_values_test index
-# - Index 9 example documents
-# - Search tests about doc_values and index
+# - 1.Create flat_object mapping for flat_object_doc_values_test index
+# - 2.Index 9 example documents
+# - 3.Search tests about doc_values and index
+# - 4.Fetch doc_value from flat_object field
 
 setup:
   - skip:
@@ -786,3 +787,48 @@ teardown:
 
   - length: { hits.hits: 1 }
   - match: { hits.hits.0._source.order: "order8" }
+
+  # Stored Fields with exact dot path.
+  - do:
+      search:
+        body: {
+          _source: false,
+          query: {
+            bool: {
+              must: [
+                {
+                  term: {
+                    order: "order0"
+                  }
+                }
+              ]
+            }
+          },
+          stored_fields: "_none_",
+          docvalue_fields: [ "issue.labels.name","order" ]
+        }
+
+  - length: { hits.hits: 1 }
+  - match: { hits.hits.0.fields: { "order" : [ "order0" ], "issue.labels.name": [ "abc0" ] } }
+
+  - do:
+      search:
+        body: {
+          _source: false,
+          query: {
+            bool: {
+              must: [
+                {
+                  term: {
+                    order: "order0"
+                  }
+                }
+              ]
+            }
+          },
+          stored_fields: "_none_",
+          docvalue_fields: [ "issue.labels.name" ]
+        }
+
+  - length: { hits.hits: 1 }
+  - match: { hits.hits.0.fields: { "issue.labels.name": [ "abc0" ] } }
diff --git a/server/src/internalClusterTest/java/org/opensearch/search/SearchTimeoutIT.java b/server/src/internalClusterTest/java/org/opensearch/search/SearchTimeoutIT.java
@@ -82,7 +82,7 @@ protected Settings nodeSettings(int nodeOrdinal) {
     }
 
     public void testSimpleTimeout() throws Exception {
-        final int numDocs = 1000;
+        final int numDocs = 10;
         for (int i = 0; i < numDocs; i++) {
             client().prepareIndex("test").setId(Integer.toString(i)).setSource("field", "value").get();
         }

diff --git a/server/src/internalClusterTest/java/org/opensearch/search/fields/SearchFieldsIT.java b/server/src/internalClusterTest/java/org/opensearch/search/fields/SearchFieldsIT.java
@@ -1023,7 +1023,7 @@ public void testDocValueFields() throws Exception {
             .startObject("ip_field")
             .field("type", "ip")
             .endObject()
-            .startObject("flat_object_field")
+            .startObject("flat_object_field1")
             .field("type", "flat_object")
             .endObject()
             .endObject()
@@ -1050,9 +1050,11 @@ public void testDocValueFields() throws Exception {
                     .field("boolean_field", true)
                     .field("binary_field", new byte[] { 42, 100 })
                     .field("ip_field", "::1")
-                    .field("flat_object_field")
+                    .field("flat_object_field1")
                     .startObject()
+                    .field("fooa", "bara")
                     .field("foo", "bar")
+                    .field("foob", "barb")
                     .endObject()
                     .endObject()
             )
@@ -1075,7 +1077,7 @@ public void testDocValueFields() throws Exception {
             .addDocValueField("boolean_field")
             .addDocValueField("binary_field")
             .addDocValueField("ip_field")
-            .addDocValueField("flat_object_field");
+            .addDocValueField("flat_object_field1.foo");
         SearchResponse searchResponse = builder.get();
 
         assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L));
@@ -1097,7 +1099,7 @@ public void testDocValueFields() throws Exception {
                     "keyword_field",
                     "binary_field",
                     "ip_field",
-                    "flat_object_field"
+                    "flat_object_field1.foo"
                 )
             )
         );
@@ -1116,7 +1118,7 @@ public void testDocValueFields() throws Exception {
         assertThat(searchResponse.getHits().getAt(0).getFields().get("keyword_field").getValue(), equalTo("foo"));
         assertThat(searchResponse.getHits().getAt(0).getFields().get("binary_field").getValue(), equalTo("KmQ"));
         assertThat(searchResponse.getHits().getAt(0).getFields().get("ip_field").getValue(), equalTo("::1"));
-        assertThat(searchResponse.getHits().getAt(0).getFields().get("flat_object_field").getValue(), equalTo("flat_object_field.foo"));
+        assertThat(searchResponse.getHits().getAt(0).getFields().get("flat_object_field1.foo").getValue(), equalTo("bar"));
 
         builder = client().prepareSearch().setQuery(matchAllQuery()).addDocValueField("*field");
         searchResponse = builder.get();
@@ -1139,8 +1141,7 @@ public void testDocValueFields() throws Exception {
                     "text_field",
                     "keyword_field",
                     "binary_field",
-                    "ip_field",
-                    "flat_object_field"
+                    "ip_field"
                 )
             )
         );
@@ -1160,7 +1161,6 @@ public void testDocValueFields() throws Exception {
         assertThat(searchResponse.getHits().getAt(0).getFields().get("keyword_field").getValue(), equalTo("foo"));
         assertThat(searchResponse.getHits().getAt(0).getFields().get("binary_field").getValue(), equalTo("KmQ"));
         assertThat(searchResponse.getHits().getAt(0).getFields().get("ip_field").getValue(), equalTo("::1"));
-        assertThat(searchResponse.getHits().getAt(0).getFields().get("flat_object_field").getValue(), equalTo("flat_object_field.foo"));
 
         builder = client().prepareSearch()
             .setQuery(matchAllQuery())
@@ -1176,7 +1176,7 @@ public void testDocValueFields() throws Exception {
             .addDocValueField("boolean_field", "use_field_mapping")
             .addDocValueField("binary_field", "use_field_mapping")
             .addDocValueField("ip_field", "use_field_mapping")
-            .addDocValueField("flat_object_field", "use_field_mapping");
+            .addDocValueField("flat_object_field1.foo", null);
         ;
         searchResponse = builder.get();
 
@@ -1199,7 +1199,7 @@ public void testDocValueFields() throws Exception {
                     "keyword_field",
                     "binary_field",
                     "ip_field",
-                    "flat_object_field"
+                    "flat_object_field1.foo"
                 )
             )
         );
@@ -1219,7 +1219,7 @@ public void testDocValueFields() throws Exception {
         assertThat(searchResponse.getHits().getAt(0).getFields().get("keyword_field").getValue(), equalTo("foo"));
         assertThat(searchResponse.getHits().getAt(0).getFields().get("binary_field").getValue(), equalTo("KmQ"));
         assertThat(searchResponse.getHits().getAt(0).getFields().get("ip_field").getValue(), equalTo("::1"));
-        assertThat(searchResponse.getHits().getAt(0).getFields().get("flat_object_field").getValue(), equalTo("flat_object_field.foo"));
+        assertThat(searchResponse.getHits().getAt(0).getFields().get("flat_object_field1.foo").getValue(), equalTo("bar"));
 
         builder = client().prepareSearch()
             .setQuery(matchAllQuery())

diff --git a/server/src/main/java/org/opensearch/index/mapper/DocValueFetcher.java b/server/src/main/java/org/opensearch/index/mapper/DocValueFetcher.java
@@ -43,6 +43,7 @@
 import java.util.List;
 
 import static java.util.Collections.emptyList;
+import static org.opensearch.index.mapper.FlatObjectFieldMapper.DOC_VALUE_NO_MATCH;
 
 /**
  * Value fetcher that loads from doc values.
@@ -70,7 +71,10 @@ public List<Object> fetchValues(SourceLookup lookup) throws IOException {
         }
         List<Object> result = new ArrayList<Object>(leaf.docValueCount());
         for (int i = 0, count = leaf.docValueCount(); i < count; ++i) {
-            result.add(leaf.nextValue());
+            Object value = leaf.nextValue();
+            if (value != DOC_VALUE_NO_MATCH) {
+                result.add(value);
+            }
         }
         return result;
     }

diff --git a/server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java
@@ -28,6 +28,7 @@
 import org.opensearch.common.unit.Fuzziness;
 import org.opensearch.common.xcontent.JsonToStringXContentParser;
 import org.opensearch.core.common.ParsingException;
+import org.opensearch.core.common.io.stream.StreamOutput;
 import org.opensearch.core.xcontent.DeprecationHandler;
 import org.opensearch.core.xcontent.NamedXContentRegistry;
 import org.opensearch.core.xcontent.XContentParser;
@@ -36,11 +37,13 @@
 import org.opensearch.index.fielddata.plain.SortedSetOrdinalsIndexFieldData;
 import org.opensearch.index.mapper.KeywordFieldMapper.KeywordFieldType;
 import org.opensearch.index.query.QueryShardContext;
+import org.opensearch.search.DocValueFormat;
 import org.opensearch.search.aggregations.support.CoreValuesSourceType;
 import org.opensearch.search.lookup.SearchLookup;
 
 import java.io.IOException;
 import java.io.UncheckedIOException;
+import java.time.ZoneId;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Iterator;
@@ -63,6 +66,7 @@
 public final class FlatObjectFieldMapper extends DynamicKeyFieldMapper {
 
     public static final String CONTENT_TYPE = "flat_object";
+    public static final String DOC_VALUE_NO_MATCH = new String("");
 
     /**
      * In flat_object field mapper, field type is similar to keyword field type
@@ -272,7 +276,7 @@ NamedAnalyzer normalizer() {
         @Override
         public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, Supplier<SearchLookup> searchLookup) {
             failIfNoDocValues();
-            return new SortedSetOrdinalsIndexFieldData.Builder(name(), CoreValuesSourceType.BYTES);
+            return new SortedSetOrdinalsIndexFieldData.Builder(valueFieldType().name(), CoreValuesSourceType.BYTES);
         }
 
         @Override
@@ -304,6 +308,30 @@ protected String parseSourceValue(Object value) {
             };
         }
 
+        @Override
+        public DocValueFormat docValueFormat(@Nullable String format, ZoneId timeZone) {
+            if (format != null) {
+                throw new IllegalArgumentException("Field [" + name() + "] of type [" + typeName() + "] does not support custom formats");
+            }
+            if (timeZone != null) {
+                throw new IllegalArgumentException(
+                    "Field [" + name() + "] of type [" + typeName() + "] does not support custom time zones"
+                );
+            }
+            if (mappedFieldTypeName != null) {
+                return new FlatObjectDocValueFormat(mappedFieldTypeName + DOT_SYMBOL + name() + EQUAL_SYMBOL);
+            } else {
+                throw new IllegalArgumentException(
+                    "Field [" + name() + "] of type [" + typeName() + "] does not support doc_value in root field"
+                );
+            }
+        }
+
+        @Override
+        public boolean isAggregatable() {
+            return false;
+        }
+
         @Override
         public Object valueForDisplay(Object value) {
             if (value == null) {
@@ -530,6 +558,39 @@ public Query wildcardQuery(
             return valueFieldType().wildcardQuery(rewriteValue(value), method, caseInsensitve, context);
         }
 
+        /**
+         * A doc_value formatter for flat_object field.
+         */
+        public class FlatObjectDocValueFormat implements DocValueFormat {
+            private static final String NAME = "flat_object";
+            private final String prefix;
+
+            public FlatObjectDocValueFormat(String prefix) {
+                this.prefix = prefix;
+            }
+
+            @Override
+            public String getWriteableName() {
+                return NAME;
+            }
+
+            @Override
+            public void writeTo(StreamOutput out) {}
+
+            @Override
+            public String format(BytesRef value) {
+                String parsedValue = inputToString(value);
+                if (parsedValue.startsWith(prefix) == false) {
+                    return DOC_VALUE_NO_MATCH;
+                }
+                return parsedValue.substring(prefix.length());
+            }
+
+            @Override
+            public BytesRef parseBytesRef(String value) {
+                return new BytesRef((String) valueFieldType.rewriteForDocValue(rewriteValue(value)));
+            }
+        }
     }
 
     private final ValueFieldMapper valueFieldMapper;

diff --git a/server/src/test/java/org/opensearch/index/mapper/FlatObjectFieldMapperTests.java b/server/src/test/java/org/opensearch/index/mapper/FlatObjectFieldMapperTests.java
@@ -21,6 +21,7 @@
 import org.opensearch.core.xcontent.ToXContent;
 import org.opensearch.core.xcontent.XContentBuilder;
 import org.opensearch.index.query.QueryShardContext;
+import org.opensearch.search.DocValueFormat;
 
 import java.io.IOException;
 
@@ -397,6 +398,27 @@ public void testDeduplicationValue() throws IOException {
         assertEquals(new BytesRef("field.labels=3"), fieldValueAndPaths[4].binaryValue());
     }
 
+    public void testFetchDocValues() throws IOException {
+        MapperService mapperService = createMapperService(fieldMapping(b -> b.field("type", "flat_object")));
+        {
+            // test valueWithPathField
+            MappedFieldType ft = mapperService.fieldType("field.name");
+            DocValueFormat format = ft.docValueFormat(null, null);
+            String storedValue = "field.field.name=1234";
+
+            Object object = format.format(new BytesRef(storedValue));
+            assertEquals("1234", object);
+        }
+
+        {
+            // test valueField
+            MappedFieldType ft = mapperService.fieldType("field");
+            Throwable throwable = assertThrows(IllegalArgumentException.class, () -> ft.docValueFormat(null, null));
+            assertEquals("Field [field] of type [flat_object] does not support doc_value in root field", throwable.getMessage());
+        }
+
+    }
+
     @Override
     protected void registerParameters(ParameterChecker checker) throws IOException {
         // In the future we will want to make sure parameter updates are covered.