Parse none type field as null instead of throw exception (#406)

Signed-off-by: penghuo <penghuo@gmail.com>
opensearch-project · Feb 2, 2022 · b0ef5e0 · b0ef5e0
1 parent 91e86b8
commit b0ef5e0
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 14 deletions.
diff --git a/...ch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java b/...ch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java
@@ -40,6 +40,7 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.function.Function;
 import lombok.Getter;
 import lombok.Setter;
@@ -130,7 +131,7 @@ public OpenSearchExprValueFactory(
   public ExprValue construct(String jsonString) {
     try {
       return parse(new OpenSearchJsonContent(OBJECT_MAPPER.readTree(jsonString)), TOP_PATH,
-          STRUCT);
+          Optional.of(STRUCT));
     } catch (JsonProcessingException e) {
       throw new IllegalStateException(String.format("invalid json: %s.", jsonString), e);
     }
@@ -149,11 +150,12 @@ public ExprValue construct(String field, Object value) {
     return parse(new ObjectContent(value), field, type(field));
   }
 
-  private ExprValue parse(Content content, String field, ExprType type) {
-    if (content.isNull()) {
+  private ExprValue parse(Content content, String field, Optional<ExprType> fieldType) {
+    if (content.isNull() || !fieldType.isPresent()) {
       return ExprNullValue.of();
     }
 
+    ExprType type = fieldType.get();
     if (type == STRUCT) {
       return parseStruct(content, field);
     } else if (type == ARRAY) {
@@ -169,12 +171,12 @@ private ExprValue parse(Content content, String field, ExprType type) {
     }
   }
 
-  private ExprType type(String field) {
-    if (typeMapping.containsKey(field)) {
-      return typeMapping.get(field);
-    } else {
-      throw new IllegalStateException(String.format("No type found for field: %s.", field));
-    }
+  /**
+   * In OpenSearch, it is possible field doesn't have type definition in mapping.
+   * but has empty value. For example, {"empty_field": []}.
+   */
+  private Optional<ExprType> type(String field) {
+    return Optional.ofNullable(typeMapping.get(field));
   }
 
   /**
@@ -223,7 +225,7 @@ private ExprValue parseStruct(Content content, String prefix) {
    */
   private ExprValue parseArray(Content content, String prefix) {
     List<ExprValue> result = new ArrayList<>();
-    content.array().forEachRemaining(v -> result.add(parse(v, prefix, STRUCT)));
+    content.array().forEachRemaining(v -> result.add(parse(v, prefix, Optional.of(STRUCT))));
     return new ExprCollectionValue(result);
   }
 

diff --git a/...rc/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java b/...rc/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java
@@ -347,10 +347,10 @@ public void constructFromInvalidJsonThrowException() {
   }
 
   @Test
-  public void noTypeFoundForMappingThrowException() {
-    IllegalStateException exception =
-        assertThrows(IllegalStateException.class, () -> tupleValue("{\"not_exist\":1}"));
-    assertEquals("No type found for field: not_exist.", exception.getMessage());
+  public void noTypeFoundForMapping() {
+    assertEquals(nullValue(), tupleValue("{\"not_exist\":[]}").get("not_exist"));
+    // Only for test coverage, It is impossible in OpenSearch.
+    assertEquals(nullValue(), tupleValue("{\"not_exist\":1}").get("not_exist"));
   }
 
   @Test