Fix foreach JSON array type coercion (opensearch-project#5637)

Signed-off-by: Songkan Tang <songkant@amazon.com>

Author: songkant-aws

core/src/main/java/org/opensearch/sql/calcite/ForeachPlanner.java

@@ -432,9 +432,9 @@ private UnresolvedExpression asArrayExpression(
    * VARCHAR.
    *
    * <p>For {@code json_array()} calls and string literals the content is visible at plan time;
-   * mixed content is rejected. For opaque expressions — typically a field holding JSON text, the
-   * primary Splunk use of json_array mode — content is unknowable, so infer from usage: an item
-   * placeholder consumed by arithmetic means numeric elements, anything else means strings.
+   * mixed content is rejected. For opaque expressions, typically a field holding JSON text, content
+   * is unknowable, so infer from usage: an item placeholder consumed by arithmetic means numeric
+   * elements, anything else means strings.
    */
   private SqlTypeName jsonElementType(
       UnresolvedExpression collection, CalcitePlanContext context, Foreach node) {

core/src/main/java/org/opensearch/sql/expression/function/jsonUDF/ForeachJsonArrayFunctionImpl.java

@@ -10,7 +10,6 @@
 import com.google.gson.JsonArray;
 import com.google.gson.JsonElement;
 import com.google.gson.JsonSyntaxException;
-import java.math.BigDecimal;
 import java.util.List;
 import java.util.stream.StreamSupport;
 import org.apache.calcite.adapter.enumerable.NotNullImplementor;
@@ -20,6 +19,7 @@
 import org.apache.calcite.linq4j.tree.Expressions;
 import org.apache.calcite.linq4j.tree.Types;
 import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.runtime.SqlFunctions;
 import org.apache.calcite.sql.type.SqlReturnTypeInference;
 import org.apache.calcite.sql.type.SqlTypeName;
 import org.apache.calcite.sql.type.SqlTypeUtil;
@@ -97,14 +97,19 @@ private static Object cast(Object value, SqlTypeName elementType) {
     if (value == null) {
       return null;
     }
-    return switch (elementType) {
-      case DOUBLE -> ((Number) value).doubleValue();
-      case VARCHAR ->
-          value instanceof List<?> || value instanceof java.util.Map<?, ?>
-              ? gson.toJson(value)
-              : String.valueOf(value);
-      case DECIMAL -> BigDecimal.valueOf(((Number) value).doubleValue());
-      default -> value;
-    };
+    // Match Calcite SAFE_CAST semantics for runtime values whose type is unknown during planning.
+    try {
+      return switch (elementType) {
+        case DOUBLE -> SqlFunctions.toDouble(value);
+        case VARCHAR ->
+            value instanceof List<?> || value instanceof java.util.Map<?, ?>
+                ? gson.toJson(value)
+                : String.valueOf(value);
+        case DECIMAL -> SqlFunctions.toBigDecimal(value);
+        default -> value;
+      };
+    } catch (RuntimeException e) {
+      return null;
+    }
   }
 }

core/src/test/java/org/opensearch/sql/expression/function/CollectionUDF/ForeachFunctionImplTest.java

@@ -40,6 +40,13 @@ public void testMalformedJsonArrayIsEmpty() {
     assertEquals(List.of(), ForeachJsonArrayFunctionImpl.eval("not-json", "VARCHAR"));
   }
 
+  @Test
+  public void testJsonArraySafelyCoercesNumericElements() {
+    assertEquals(
+        Arrays.asList(10.0, 20.0, null),
+        ForeachJsonArrayFunctionImpl.eval("[10,\"20\",\"not-a-number\"]", "DOUBLE"));
+  }
+
   @Test
   public void testStatePreservesHeterogeneousAndNullSlots() {
     assertEquals(

docs/user/ppl/cmd/foreach.md

@@ -41,9 +41,9 @@ Placeholders renamed via `itemstr`/`iterstr` may be written without the `<<...>>
 The following considerations apply when using the `foreach` command:
 
 * In collection modes, the bracketed `eval` acts as an accumulator: each target field must already exist (typically initialized with a preceding `eval`), and the expressions are applied once per element. Multiple assignments run from left to right, so a later assignment in the same iteration sees an earlier assignment's updated value.
-* In `json_array` mode the element type is inferred: a `json_array(...)` call or JSON string literal is inspected at plan time; for a field holding JSON text, elements are treated as numbers when `<<ITEM>>` is used in arithmetic and as strings otherwise. Mixed string/number JSON arrays are rejected.
+* In `json_array` mode the element type is inferred: a `json_array(...)` call or JSON string literal is inspected at plan time; for a field holding JSON text, elements are treated as numbers when `<<ITEM>>` is used in arithmetic and as strings otherwise. Plan-time arrays with mixed string/number elements are rejected. When numeric use is inferred for field-backed JSON text, elements that cannot be converted to numbers evaluate to `null`.
 * Placeholders are also substituted inside string literals. For example, `eval <<FIELD>> = '<<FIELD>>'` replaces each selected field value with that field's name.
-* As in Splunk, `multivalue` mode applied to a non-array value and `json_array` mode applied to a native array are no-ops. A field whose mapping is scalar cannot be identified as multivalue at plan time even when a document stores several values, so `multivalue` mode also no-ops for that mapping.
+* `multivalue` mode applied to a non-array value and `json_array` mode applied to a native array are no-ops. A field whose mapping is scalar cannot be identified as multivalue at plan time even when a document stores several values, so `multivalue` mode also no-ops for that mapping.
 
 ## Example 1: Apply the same calculation to multiple fields
 

docs/user/ppl/index.md

@@ -43,6 +43,7 @@ source=accounts
 | [fields command](cmd/fields.md) | 1.0 | stable (since 1.0) | Keep or remove fields from the search result. |
 | [rename command](cmd/rename.md) | 1.0 | stable (since 1.0) | Rename one or more fields in the search result. |
 | [eval command](cmd/eval.md) | 1.0 | stable (since 1.0) | Evaluate an expression and append the result to the search result. |
+| [foreach command](cmd/foreach.md) | 3.8 | experimental (since 3.8) | Run a templated evaluation for each selected field or collection element. |
 | [convert command](cmd/convert.md) | 3.5 | experimental (since 3.5) | Transform field values to numeric values using specialized conversion functions. |
 | [replace command](cmd/replace.md) | 3.4 | experimental (since 3.4) | Replace text in one or more fields in the search result |
 | [fillnull command](cmd/fillnull.md) | 3.0 | experimental (since 3.0) | Fill null with provided value in one or more fields in the search result. |

integ-test/src/test/java/org/opensearch/sql/calcite/CalciteNoPushdownIT.java

@@ -38,6 +38,7 @@
   CalciteExpandCommandIT.class,
   CalciteFieldFormatCommandIT.class,
   CalciteForeachCommandIT.class,
+  ForeachFieldJsonIT.class,
   CalciteFieldsCommandIT.class,
   CalciteFillNullCommandIT.class,
   CalciteFlattenCommandIT.class,

integ-test/src/test/java/org/opensearch/sql/calcite/remote/ForeachFieldJsonIT.java

@@ -17,7 +17,7 @@
 import org.opensearch.sql.legacy.TestUtils;
 import org.opensearch.sql.ppl.PPLIntegTestCase;
 
-/** Foreach collection modes over index fields (Splunk-parity scenarios). */
+/** Foreach collection modes over index fields. */
 public class ForeachFieldJsonIT extends PPLIntegTestCase {
 
   @Override
@@ -42,7 +42,6 @@ public void init() throws Exception {
 
   @Test
   public void testJsonArrayModeOnFieldWithNumericContent() throws IOException {
-    // Splunk: field holding "[10,20,30]" with foreach mode=json_array sums to 60.
     JSONObject result =
         executeQuery(
             "source=test_foreach_field2 | eval total = 0 | foreach mode=json_array jsonfield ["
@@ -81,6 +80,16 @@ public void testJsonArrayModeOnFieldWithStringContent() throws IOException {
     verifyDataRows(result, rows("ab"));
   }
 
+  @Test
+  public void testJsonArrayFieldSafelyCoercesNonNumericItems() throws IOException {
+    JSONObject result =
+        executeQuery(
+            "source=test_foreach_field2 | eval total = 0 | foreach mode=json_array jsonstrs ["
+                + " eval total = total + <<ITEM>> ] | fields total");
+    verifySchema(result, schema("total", "double"));
+    verifyDataRows(result, rows((Object) null));
+  }
+
   /**
    * Native OpenSearch array fields (a long field holding [1,2,3]) are typed as scalar BIGINT at
    * plan time because OpenSearch mappings do not distinguish scalars from arrays. foreach
@@ -111,7 +120,7 @@ public void testNestedFieldMultivalueIterates() throws IOException {
     verifyDataRows(result, rows(2));
   }
 
-  /** Splunk silently no-ops when a mode is fed the wrong collection shape. */
+  /** Collection modes are no-ops when the input has a different collection shape. */
   @Test
   public void testJsonArrayModeOnRealArrayIsNoOp() throws IOException {
     JSONObject result =