Array values are preserved (opensearch-project#1300) (opensearch-proj…

…ect#3095) (opensearch-project#3120) Signed-off-by: Norman Jordan <norman.jordan@improving.com> (cherry picked from commit e109417) Co-authored-by: normanj-bitquill <78755797+normanj-bitquill@users.noreply.github.com> Signed-off-by: Louis Chu <clingzhi@amazon.com>
noCharger · Oct 25, 2024 · f81d558 · f81d558
1 parent 0d2a4aa
commit f81d558
Show file tree

Hide file tree

Showing 19 changed files with 285 additions and 64 deletions.
diff --git a/common/src/main/java/org/opensearch/sql/common/setting/Settings.java b/common/src/main/java/org/opensearch/sql/common/setting/Settings.java
@@ -27,6 +27,9 @@ public enum Key {
     /** PPL Settings. */
     PPL_ENABLED("plugins.ppl.enabled"),
 
+    /** Query Settings. */
+    FIELD_TYPE_TOLERANCE("plugins.query.field_type_tolerance"),
+
     /** Common Settings for SQL and PPL. */
     QUERY_MEMORY_LIMIT("plugins.query.memory_limit"),
     QUERY_SIZE_LIMIT("plugins.query.size_limit"),

diff --git a/docs/user/beyond/partiql.rst b/docs/user/beyond/partiql.rst
@@ -202,11 +202,11 @@ Selecting top level for object fields, object fields of array value and nested f
 
     os> SELECT city, accounts, projects FROM people;
     fetched rows / total rows = 1/1
-    +-----------------------------------------------------+------------+----------------------------------------------------------------------------------------------------------------+
-    | city                                                | accounts   | projects                                                                                                       |
-    |-----------------------------------------------------+------------+----------------------------------------------------------------------------------------------------------------|
-    | {'name': 'Seattle', 'location': {'latitude': 10.5}} | {'id': 1}  | [{'name': 'AWS Redshift Spectrum querying'},{'name': 'AWS Redshift security'},{'name': 'AWS Aurora security'}] |
-    +-----------------------------------------------------+------------+----------------------------------------------------------------------------------------------------------------+
+    +-----------------------------------------------------+-----------------------+----------------------------------------------------------------------------------------------------------------+
+    | city                                                | accounts              | projects                                                                                                       |
+    |-----------------------------------------------------+-----------------------+----------------------------------------------------------------------------------------------------------------|
+    | {'name': 'Seattle', 'location': {'latitude': 10.5}} | [{'id': 1},{'id': 2}] | [{'name': 'AWS Redshift Spectrum querying'},{'name': 'AWS Redshift security'},{'name': 'AWS Aurora security'}] |
+    +-----------------------------------------------------+-----------------------+----------------------------------------------------------------------------------------------------------------+
 
 Example 2: Selecting Deeper Levels
 ----------------------------------

diff --git a/docs/user/ppl/general/datatypes.rst b/docs/user/ppl/general/datatypes.rst
@@ -412,8 +412,8 @@ Select deeper level for object fields of array value which returns the first ele
 
     os> source = people | fields accounts, accounts.id;
     fetched rows / total rows = 1/1
-    +------------+---------------+
-    | accounts   | accounts.id   |
-    |------------+---------------|
-    | {'id': 1}  | 1             |
-    +------------+---------------+
+    +-----------------------+-------------+
+    | accounts              | accounts.id |
+    |-----------------------+-------------|
+    | [{'id': 1},{'id': 2}] | 1           |
+    +-----------------------+-------------+
diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/ObjectFieldSelectIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/ObjectFieldSelectIT.java
@@ -14,6 +14,7 @@
 import org.json.JSONArray;
 import org.json.JSONObject;
 import org.junit.Test;
+import org.opensearch.sql.common.setting.Settings;
 import org.opensearch.sql.legacy.utils.StringUtils;
 
 /**
@@ -79,9 +80,20 @@ public void testSelectNestedFieldItself() {
   @Test
   public void testSelectObjectFieldOfArrayValuesItself() {
     JSONObject response = new JSONObject(query("SELECT accounts FROM %s"));
+    verifyDataRows(response, rows(new JSONArray("[{\"id\":1},{\"id\":2}]")));
+  }
 
-    // Only the first element of the list of is returned.
-    verifyDataRows(response, rows(new JSONObject("{\"id\": 1}")));
+  @Test
+  public void testSelectObjectFieldOfArrayValuesItselfNoFieldTypeTolerance() throws Exception {
+    updateClusterSettings(
+        new ClusterSetting(PERSISTENT, Settings.Key.FIELD_TYPE_TOLERANCE.getKeyValue(), "false"));
+    try {
+      JSONObject response = new JSONObject(query("SELECT accounts FROM %s"));
+      verifyDataRows(response, rows(new JSONObject("{\"id\":1}")));
+    } finally {
+      updateClusterSettings(
+          new ClusterSetting(PERSISTENT, Settings.Key.FIELD_TYPE_TOLERANCE.getKeyValue(), "true"));
+    }
   }
 
   @Test

diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/StandaloneIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/StandaloneIT.java
@@ -149,8 +149,11 @@ public void onFailure(Exception e) {
 
   private Settings defaultSettings() {
     return new Settings() {
-      private final Map<Key, Integer> defaultSettings =
-          new ImmutableMap.Builder<Key, Integer>().put(Key.QUERY_SIZE_LIMIT, 200).build();
+      private final Map<Key, Object> defaultSettings =
+          new ImmutableMap.Builder<Key, Object>()
+              .put(Key.QUERY_SIZE_LIMIT, 200)
+              .put(Key.FIELD_TYPE_TOLERANCE, true)
+              .build();
 
       @Override
       public <T> T getSettingValue(Key key) {

diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/NestedIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/NestedIT.java
@@ -423,7 +423,7 @@ public void test_nested_in_where_as_predicate_expression_with_multiple_condition
             + " nested(message.dayOfWeek) >= 4";
     JSONObject result = executeJdbcRequest(query);
     assertEquals(2, result.getInt("total"));
-    verifyDataRows(result, rows("c", "ab", 4), rows("zz", "aa", 6));
+    verifyDataRows(result, rows("c", "ab", 4), rows("zz", new JSONArray(List.of("aa", "bb")), 6));
   }
 
   @Test

diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/StandalonePaginationIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/StandalonePaginationIT.java
@@ -166,6 +166,7 @@ private Settings defaultSettings() {
           new ImmutableMap.Builder<Key, Object>()
               .put(Key.QUERY_SIZE_LIMIT, 200)
               .put(Key.SQL_CURSOR_KEEP_ALIVE, TimeValue.timeValueMinutes(1))
+              .put(Key.FIELD_TYPE_TOLERANCE, true)
               .build();
 
       @Override

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
@@ -75,6 +75,9 @@ public class OpenSearchExprValueFactory {
   /** The Mapping of Field and ExprType. */
   private final Map<String, OpenSearchDataType> typeMapping;
 
+  /** Whether to support nested value types (such as arrays) */
+  private final boolean fieldTypeTolerance;
+
   /**
    * Extend existing mapping by new data without overwrite. Called from aggregation only {@see
    * AggregationQueryBuilder#buildTypeMapping}.
@@ -143,8 +146,10 @@ public void extendTypeMapping(Map<String, OpenSearchDataType> typeMapping) {
           .build();
 
   /** Constructor of OpenSearchExprValueFactory. */
-  public OpenSearchExprValueFactory(Map<String, OpenSearchDataType> typeMapping) {
+  public OpenSearchExprValueFactory(
+      Map<String, OpenSearchDataType> typeMapping, boolean fieldTypeTolerance) {
     this.typeMapping = OpenSearchDataType.traverseAndFlatten(typeMapping);
+    this.fieldTypeTolerance = fieldTypeTolerance;
   }
 
   /**
@@ -160,7 +165,7 @@ public ExprValue construct(String jsonString, boolean supportArrays) {
           new OpenSearchJsonContent(OBJECT_MAPPER.readTree(jsonString)),
           TOP_PATH,
           Optional.of(STRUCT),
-          supportArrays);
+          fieldTypeTolerance || supportArrays);
     } catch (JsonProcessingException e) {
       throw new IllegalStateException(String.format("invalid json: %s.", jsonString), e);
     }

diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchScrollRequest.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchScrollRequest.java
@@ -178,6 +178,8 @@ public OpenSearchScrollRequest(StreamInput in, OpenSearchStorageEngine engine)
     includes = in.readStringList();
     indexName = new IndexName(in);
     OpenSearchIndex index = (OpenSearchIndex) engine.getTable(null, indexName.toString());
-    exprValueFactory = new OpenSearchExprValueFactory(index.getFieldOpenSearchTypes());
+    exprValueFactory =
+        new OpenSearchExprValueFactory(
+            index.getFieldOpenSearchTypes(), index.isFieldTypeTolerance());
   }
 }
diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java
@@ -222,6 +222,13 @@ public class OpenSearchSettings extends Settings {
           Setting.Property.NodeScope,
           Setting.Property.Dynamic);
 
+  public static final Setting<?> FIELD_TYPE_TOLERANCE_SETTING =
+      Setting.boolSetting(
+          Key.FIELD_TYPE_TOLERANCE.getKeyValue(),
+          true,
+          Setting.Property.NodeScope,
+          Setting.Property.Dynamic);
+
   /** Construct OpenSearchSetting. The OpenSearchSetting must be singleton. */
   @SuppressWarnings("unchecked")
   public OpenSearchSettings(ClusterSettings clusterSettings) {
@@ -359,13 +366,19 @@ public OpenSearchSettings(ClusterSettings clusterSettings) {
         clusterSettings,
         Key.SESSION_INACTIVITY_TIMEOUT_MILLIS,
         SESSION_INACTIVITY_TIMEOUT_MILLIS_SETTING,
-        new Updater((Key.SESSION_INACTIVITY_TIMEOUT_MILLIS)));
+        new Updater(Key.SESSION_INACTIVITY_TIMEOUT_MILLIS));
     register(
         settingBuilder,
         clusterSettings,
         Key.STREAMING_JOB_HOUSEKEEPER_INTERVAL,
         STREAMING_JOB_HOUSEKEEPER_INTERVAL_SETTING,
-        new Updater((Key.STREAMING_JOB_HOUSEKEEPER_INTERVAL)));
+        new Updater(Key.STREAMING_JOB_HOUSEKEEPER_INTERVAL));
+    register(
+        settingBuilder,
+        clusterSettings,
+        Key.FIELD_TYPE_TOLERANCE,
+        FIELD_TYPE_TOLERANCE_SETTING,
+        new Updater(Key.FIELD_TYPE_TOLERANCE));
     defaultSettings = settingBuilder.build();
   }
 
@@ -441,6 +454,7 @@ public static List<Setting<?>> pluginSettings() {
         .add(DATASOURCES_LIMIT_SETTING)
         .add(SESSION_INACTIVITY_TIMEOUT_MILLIS_SETTING)
         .add(STREAMING_JOB_HOUSEKEEPER_INTERVAL_SETTING)
+        .add(FIELD_TYPE_TOLERANCE_SETTING)
         .build();
   }
 

diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndex.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndex.java
@@ -177,7 +177,12 @@ private OpenSearchExprValueFactory createExprValueFactory() {
     Map<String, OpenSearchDataType> allFields = new HashMap<>();
     getReservedFieldTypes().forEach((k, v) -> allFields.put(k, OpenSearchDataType.of(v)));
     allFields.putAll(getFieldOpenSearchTypes());
-    return new OpenSearchExprValueFactory(allFields);
+    return new OpenSearchExprValueFactory(
+        allFields, settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE));
+  }
+
+  public boolean isFieldTypeTolerance() {
+    return settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE);
   }
 
   @VisibleForTesting

diff --git a/...rch/src/main/java/org/opensearch/sql/opensearch/storage/script/core/ExpressionScript.java b/...rch/src/main/java/org/opensearch/sql/opensearch/storage/script/core/ExpressionScript.java
@@ -102,7 +102,7 @@ private OpenSearchExprValueFactory buildValueFactory(Set<ReferenceExpression> fi
     Map<String, OpenSearchDataType> typeEnv =
         fields.stream()
             .collect(toMap(ReferenceExpression::getAttr, e -> OpenSearchDataType.of(e.type())));
-    return new OpenSearchExprValueFactory(typeEnv);
+    return new OpenSearchExprValueFactory(typeEnv, false);
   }
 
   private Environment<Expression, ExprValue> buildValueEnv(

diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClientTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClientTest.java
@@ -295,7 +295,6 @@ void search() {
             new SearchHits(
                 new SearchHit[] {searchHit}, new TotalHits(1L, TotalHits.Relation.EQUAL_TO), 1.0F));
     when(searchHit.getSourceAsString()).thenReturn("{\"id\", 1}");
-    when(searchHit.getInnerHits()).thenReturn(null);
     when(factory.construct(any(), anyBoolean())).thenReturn(exprTupleValue);
 
     // Mock second scroll request followed

diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchRestClientTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchRestClientTest.java
@@ -282,7 +282,6 @@ void search() throws IOException {
             new SearchHits(
                 new SearchHit[] {searchHit}, new TotalHits(1L, TotalHits.Relation.EQUAL_TO), 1.0F));
     when(searchHit.getSourceAsString()).thenReturn("{\"id\", 1}");
-    when(searchHit.getInnerHits()).thenReturn(null);
     when(factory.construct(any(), anyBoolean())).thenReturn(exprTupleValue);
 
     // Mock second scroll request followed