From 4b204673c02d7da700637569ef74755eae043213 Mon Sep 17 00:00:00 2001 From: Songkan Tang Date: Fri, 17 Jul 2026 11:44:50 +0800 Subject: [PATCH] Fix foreach JSON array type coercion Signed-off-by: Songkan Tang --- .../sql/calcite/ForeachPlanner.java | 6 ++--- .../jsonUDF/ForeachJsonArrayFunctionImpl.java | 25 +++++++++++-------- .../ForeachFunctionImplTest.java | 7 ++++++ docs/user/ppl/cmd/foreach.md | 4 +-- docs/user/ppl/index.md | 1 + .../sql/calcite/CalciteNoPushdownIT.java | 1 + .../calcite/remote/ForeachFieldJsonIT.java | 15 ++++++++--- 7 files changed, 41 insertions(+), 18 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/calcite/ForeachPlanner.java b/core/src/main/java/org/opensearch/sql/calcite/ForeachPlanner.java index d9ef71b2220..25a8ddef642 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/ForeachPlanner.java +++ b/core/src/main/java/org/opensearch/sql/calcite/ForeachPlanner.java @@ -432,9 +432,9 @@ private UnresolvedExpression asArrayExpression( * VARCHAR. * *

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) { diff --git a/core/src/main/java/org/opensearch/sql/expression/function/jsonUDF/ForeachJsonArrayFunctionImpl.java b/core/src/main/java/org/opensearch/sql/expression/function/jsonUDF/ForeachJsonArrayFunctionImpl.java index 28dff66a8ae..df8783a311e 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/jsonUDF/ForeachJsonArrayFunctionImpl.java +++ b/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; + } } } diff --git a/core/src/test/java/org/opensearch/sql/expression/function/CollectionUDF/ForeachFunctionImplTest.java b/core/src/test/java/org/opensearch/sql/expression/function/CollectionUDF/ForeachFunctionImplTest.java index e3c3724e0aa..5a633a1a9bb 100644 --- a/core/src/test/java/org/opensearch/sql/expression/function/CollectionUDF/ForeachFunctionImplTest.java +++ b/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( diff --git a/docs/user/ppl/cmd/foreach.md b/docs/user/ppl/cmd/foreach.md index 45dcb939f8a..ba6b758fa5d 100644 --- a/docs/user/ppl/cmd/foreach.md +++ b/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 `<>` 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 `<>` 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 <> = '<>'` 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 diff --git a/docs/user/ppl/index.md b/docs/user/ppl/index.md index 939684c0ecc..b8ab58fca5a 100644 --- a/docs/user/ppl/index.md +++ b/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. | diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/CalciteNoPushdownIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/CalciteNoPushdownIT.java index 801d56fd49d..e7c024e49fa 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/CalciteNoPushdownIT.java +++ b/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, diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/ForeachFieldJsonIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/ForeachFieldJsonIT.java index 2fea0858abd..08243595fd8 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/ForeachFieldJsonIT.java +++ b/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 + <> ] | 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 =