From cc9a65cde95f170d56b0ead6c24b52ffaa31db56 Mon Sep 17 00:00:00 2001 From: Tanvir Hossain <1593367+tanvir4se@users.noreply.github.com> Date: Wed, 28 Oct 2026 11:50:34 +1100 Subject: [PATCH] [BugFix] Return 400 for mvjoin and nomv on arrays of non-string elements (#5898) Signed-off-by: Tanvir Hossain <1593367+tanvir4se@users.noreply.github.com> --- .../expression/function/PPLFuncImpTable.java | 37 +- docs/user/ppl/cmd/nomv.md | 1 + docs/user/ppl/functions/collection.md | 22 +- .../remote/CalciteArrayFunctionIT.java | 65 +++ .../calcite/remote/CalciteNoMvCommandIT.java | 39 +- .../rest-api-spec/test/issues/5898.yml | 385 ++++++++++++++++++ .../calcite/CalcitePPLFunctionTypeTest.java | 36 ++ .../sql/ppl/calcite/CalcitePPLNoMvTest.java | 22 +- 8 files changed, 592 insertions(+), 15 deletions(-) create mode 100644 integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/5898.yml diff --git a/core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java b/core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java index 77e24642fcb..597f81c89b3 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java @@ -320,6 +320,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.sql.calcite.CalcitePlanContext; +import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory; import org.opensearch.sql.calcite.utils.PPLOperandTypes; import org.opensearch.sql.calcite.utils.PlanUtils; import org.opensearch.sql.calcite.utils.UserDefinedFunctionUtils; @@ -909,6 +910,27 @@ private static RexNode[] widenIntegerOperands(RexBuilder builder, RexNode... arg return widened; } + /** + * Rejects arrays whose element type is known and is not a string. Character types (which + * include the VARCHAR-backed date, time and timestamp types), {@code ANY} and {@code NULL} + * element types are accepted. + * + * @throws IllegalArgumentException so that the caller reports a client error (400) + */ + private static void requireStringArrayElements(final RelDataType arrayType) { + final RelDataType elementType = arrayType.getComponentType(); + if (elementType == null + || SqlTypeUtil.isCharacter(elementType) + || elementType.getSqlTypeName() == SqlTypeName.ANY + || elementType.getSqlTypeName() == SqlTypeName.NULL) { + return; + } + throw new IllegalArgumentException( + String.format( + "MVJOIN only supports string arrays, but got an array of %s", + OpenSearchTypeFactory.convertRelDataTypeToExprType(elementType))); + } + private static boolean referencesLambdaParameter(RexNode node) { if (node instanceof RexLambdaRef) { return true; @@ -1247,8 +1269,19 @@ void populate() { builder.makeCast(TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR, true), source), PPLTypeChecker.family(SqlTypeFamily.ANY)); - // Register MVJOIN to use Calcite's ARRAY_JOIN - registerOperator(MVJOIN, SqlLibraryOperators.ARRAY_JOIN); + // Register MVJOIN to use Calcite's ARRAY_JOIN. The family type checker compares only the + // top-level ARRAY family, so the element type is checked here, during analysis, instead of + // failing at run time inside SqlFunctions.arrayToString. + register( + MVJOIN, + (final RexBuilder builder, final RexNode... args) -> { + requireStringArrayElements(args[0].getType()); + return builder.makeCall(SqlLibraryOperators.ARRAY_JOIN, args); + }, + wrapSqlOperandTypeChecker( + SqlLibraryOperators.ARRAY_JOIN.getOperandTypeChecker(), + SqlLibraryOperators.ARRAY_JOIN.getName(), + false)); // Register SPLIT with custom logic for empty delimiter // Case 1: Delimiter is not empty string, use SPLIT diff --git a/docs/user/ppl/cmd/nomv.md b/docs/user/ppl/cmd/nomv.md index 245abdc553a..5d0a1bd2736 100644 --- a/docs/user/ppl/cmd/nomv.md +++ b/docs/user/ppl/cmd/nomv.md @@ -57,6 +57,7 @@ The `nomv` command has the following limitations: - The `nomv` command is only available when the Apache Calcite query engine is enabled. - The newline delimiter (`\n`) is fixed and cannot be customized. For custom delimiters, use the [`mvjoin`](../functions/collection.md/#mvjoin) function directly in an [`eval`](./eval.md) expression. - `NULL` values within the array are automatically filtered out and do not appear in the output. +- The array elements must be strings. Arrays with elements of other types are rejected with an error, so cast the elements to strings first (for example, with `cast(... as string)`). ## Related commands diff --git a/docs/user/ppl/functions/collection.md b/docs/user/ppl/functions/collection.md index 006a49ef6d5..59d2afb9d67 100644 --- a/docs/user/ppl/functions/collection.md +++ b/docs/user/ppl/functions/collection.md @@ -307,7 +307,7 @@ fetched rows / total rows = 1/1 **Usage**: `mvjoin(array, delimiter)` -Joins string array elements into a single string, separated by the specified delimiter. `NULL` elements are excluded from the output. Only string arrays are supported. +Joins string array elements into a single string, separated by the specified delimiter. `NULL` elements are excluded from the output. Only string arrays are supported: arrays whose elements are not strings are rejected with an error. To join other types, cast the elements to strings first, for example with `cast(... as string)`. **Parameters**: @@ -358,6 +358,26 @@ fetched rows / total rows = 1/1 | Amber, Duke | +-------------+ ``` + +The following example casts numbers to strings before joining them: + +```ppl +source=people +| eval result = mvjoin(array(cast(1 as string), cast(2 as string), cast(3 as string)), ',') +| fields result +| head 1 +``` + +The query returns the following results: + +```text +fetched rows / total rows = 1/1 ++--------+ +| result | +|--------| +| 1,2,3 | ++--------+ +``` ## MVAPPEND diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteArrayFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteArrayFunctionIT.java index c15a129b663..19a87cc3362 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteArrayFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteArrayFunctionIT.java @@ -433,6 +433,71 @@ public void testMvjoinWithMultipleRealFields() throws IOException { firstRow.getString(3)); } + @Test + public void testMvjoinRejectsIntegerArray() { + verifyMvjoinRejectsNonStringArray("array(age)", "INTEGER"); + } + + @Test + public void testMvjoinRejectsLongArray() { + verifyMvjoinRejectsNonStringArray("array(balance)", "LONG"); + } + + @Test + public void testMvjoinRejectsBooleanArray() { + verifyMvjoinRejectsNonStringArray("array(male)", "BOOLEAN"); + } + + @Test + public void testMvjoinAcceptsDateArray() throws IOException { + // birthdate is a date field. Date, time and timestamp values are string-backed, so they stay + // accepted. The rendered value is only checked by its date part to stay format-agnostic. + final JSONObject actual = + executeQuery( + String.format( + "source=%s | where account_number = 1 | eval result = mvjoin(array(birthdate), ',')" + + " | fields result", + TEST_INDEX_BANK)); + + verifySchema(actual, schema("result", "string")); + final JSONArray dataRows = actual.getJSONArray("datarows"); + assertEquals(1, dataRows.length()); + assertTrue(dataRows.getJSONArray(0).getString(0).startsWith("2017-10-23")); + } + + @Test + public void testMvjoinAcceptsCastNumberToStringArray() throws IOException { + final JSONObject actual = + executeQuery( + String.format( + "source=%s | where account_number = 1 | eval result = mvjoin(array(cast(age as" + + " string)), ',') | fields result", + TEST_INDEX_BANK)); + + verifySchema(actual, schema("result", "string")); + verifyDataRows(actual, rows("32")); + } + + private void verifyMvjoinRejectsNonStringArray( + final String arrayExpression, final String expectedElementType) { + final Class expectedException = + isStandaloneTest() ? RuntimeException.class : ResponseException.class; + final Exception e = + assertThrows( + expectedException, + () -> + executeQuery( + String.format( + "source=%s | eval result = mvjoin(%s, ',') | fields result | head 1", + TEST_INDEX_BANK, arrayExpression))); + + verifyErrorMessageContains( + e, "MVJOIN only supports string arrays, but got an array of " + expectedElementType); + if (e instanceof ResponseException) { + assertEquals(400, ((ResponseException) e).getResponse().getStatusLine().getStatusCode()); + } + } + @Test public void testMvindexSingleElementPositive() throws IOException { JSONObject actual = diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteNoMvCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteNoMvCommandIT.java index 9e964dbc72c..fdce35a5feb 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteNoMvCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteNoMvCommandIT.java @@ -255,13 +255,14 @@ public void testNoMvWithNullAtBeginningAndEnd() throws IOException { @Test public void testNoMvWithAllNulls() throws IOException { - String q = + // age is an integer field, so it is cast to string first: nomv rejects numeric arrays. + final String q = "source=" + TEST_INDEX_BANK_WITH_NULL_VALUES - + " | where account_number = 25 | eval arr = array(age, age, age) | nomv arr | fields" - + " account_number, arr"; + + " | where account_number = 25 | eval age_str = cast(age as string) | eval arr =" + + " array(age_str, age_str, age_str) | nomv arr | fields account_number, arr"; - JSONObject result = executeQuery(q); + final JSONObject result = executeQuery(q); verifySchema(result, schema("account_number", null, "bigint"), schema("arr", null, "string")); @@ -270,19 +271,41 @@ public void testNoMvWithAllNulls() throws IOException { @Test public void testNoMvArrayWithAllNulls() throws IOException { - String q = + // age is an integer field, so it is cast to string first: nomv rejects numeric arrays. + final String q = "source=" + TEST_INDEX_BANK_WITH_NULL_VALUES - + " | where account_number = 25 | eval arr = array(age, age, age) | nomv arr | fields" - + " account_number, arr"; + + " | where account_number = 25 | eval arr = array(cast(age as string), cast(age as" + + " string), cast(age as string)) | nomv arr | fields account_number, arr"; - JSONObject result = executeQuery(q); + final JSONObject result = executeQuery(q); verifySchema(result, schema("account_number", null, "bigint"), schema("arr", null, "string")); verifyDataRows(result, rows(25, "")); } + @Test + public void testNoMvNumericArrayError() { + final ResponseException ex = + Assertions.assertThrows( + ResponseException.class, + () -> + executeQuery( + "source=" + + TEST_INDEX_BANK + + " | eval arr = array(age) | nomv arr | head 1 | fields arr")); + + final int status = ex.getResponse().getStatusLine().getStatusCode(); + Assertions.assertEquals(400, status, "Expected 400 for an array of non-string elements"); + + final String msg = ex.getMessage(); + Assertions.assertTrue( + msg.contains("MVJOIN only supports string arrays, but got an array of INTEGER"), msg); + Assertions.assertTrue(msg.contains("ExpressionEvaluationException"), msg); + Assertions.assertTrue(msg.contains("analyzing"), msg); + } + @Test public void testNoMvMultipleRowsRowLocalBehavior() throws IOException { String q = diff --git a/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/5898.yml b/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/5898.yml new file mode 100644 index 00000000000..09a76955723 --- /dev/null +++ b/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/5898.yml @@ -0,0 +1,385 @@ +# Issue: https://github.com/opensearch-project/sql/issues/5898 +# mvjoin, and nomv (which rewrites to mvjoin), returned a 500 at execution when the array's elements +# were not strings, because Calcite's ARRAY_JOIN threw while converting them. A known non-string +# element type is now rejected while analyzing, so the client sees a 400. +# +# Date, time and timestamp values are string-backed and stay accepted, as do keyword and text fields, +# mixed arrays such as array('a', n) (widened to strings) and cast(n as string). +setup: + - do: + query.settings: + body: + transient: + plugins.calcite.enabled : true + - do: + indices.create: + index: mvjoin_non_string_array_5898 + body: + mappings: + properties: + k: + type: keyword + t: + type: text + d: + type: date + i: + type: integer + l: + type: long + db: + type: double + bo: + type: boolean + ip: + type: ip + gp: + type: geo_point + o: + properties: + a: + type: keyword + ne: + type: nested + properties: + a: + type: keyword + - do: + bulk: + index: mvjoin_non_string_array_5898 + refresh: true + body: + - '{"index": {"_id": "1"}}' + - '{"k": "a", "t": "a", "d": "2024-01-01", "i": 3, "l": 3, "db": 1.5, "bo": true, "ip": "10.0.0.1", "gp": {"lat": 1, "lon": 2}, "o": {"a": "x"}, "ne": [{"a": "x"}]}' + +--- +teardown: + - do: + query.settings: + body: + transient: + plugins.calcite.enabled : false + - do: + indices.delete: + index: mvjoin_non_string_array_5898 + ignore_unavailable: true + +--- +"mvjoin and nomv on an array of an integer field are rejected while analyzing": + - skip: + features: + - headers + - do: + catch: bad_request + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = mvjoin(array(i), ',') | fields x" + - match: { "$body": "/ExpressionEvaluationException/" } + - match: { "$body": "/analyzing/" } + - match: { "$body": "/only\\s+supports\\s+string\\s+arrays/" } + - do: + catch: bad_request + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = array(i) | nomv x | fields x" + - match: { "$body": "/ExpressionEvaluationException/" } + - match: { "$body": "/analyzing/" } + - match: { "$body": "/only\\s+supports\\s+string\\s+arrays/" } + +--- +"mvjoin and nomv on an array of a long field are rejected while analyzing": + - skip: + features: + - headers + - do: + catch: bad_request + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = mvjoin(array(l), ',') | fields x" + - match: { "$body": "/ExpressionEvaluationException/" } + - match: { "$body": "/analyzing/" } + - match: { "$body": "/only\\s+supports\\s+string\\s+arrays/" } + - do: + catch: bad_request + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = array(l) | nomv x | fields x" + - match: { "$body": "/ExpressionEvaluationException/" } + - match: { "$body": "/analyzing/" } + - match: { "$body": "/only\\s+supports\\s+string\\s+arrays/" } + +--- +"mvjoin and nomv on an array of a double field are rejected while analyzing": + - skip: + features: + - headers + - do: + catch: bad_request + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = mvjoin(array(db), ',') | fields x" + - match: { "$body": "/ExpressionEvaluationException/" } + - match: { "$body": "/analyzing/" } + - match: { "$body": "/only\\s+supports\\s+string\\s+arrays/" } + - do: + catch: bad_request + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = array(db) | nomv x | fields x" + - match: { "$body": "/ExpressionEvaluationException/" } + - match: { "$body": "/analyzing/" } + - match: { "$body": "/only\\s+supports\\s+string\\s+arrays/" } + +--- +"mvjoin and nomv on an array of a boolean field are rejected while analyzing": + - skip: + features: + - headers + - do: + catch: bad_request + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = mvjoin(array(bo), ',') | fields x" + - match: { "$body": "/ExpressionEvaluationException/" } + - match: { "$body": "/analyzing/" } + - match: { "$body": "/only\\s+supports\\s+string\\s+arrays/" } + - do: + catch: bad_request + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = array(bo) | nomv x | fields x" + - match: { "$body": "/ExpressionEvaluationException/" } + - match: { "$body": "/analyzing/" } + - match: { "$body": "/only\\s+supports\\s+string\\s+arrays/" } + +--- +"mvjoin and nomv on an array of an ip field are rejected while analyzing": + - skip: + features: + - headers + - do: + catch: bad_request + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = mvjoin(array(ip), ',') | fields x" + - match: { "$body": "/ExpressionEvaluationException/" } + - match: { "$body": "/analyzing/" } + - match: { "$body": "/only\\s+supports\\s+string\\s+arrays/" } + - do: + catch: bad_request + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = array(ip) | nomv x | fields x" + - match: { "$body": "/ExpressionEvaluationException/" } + - match: { "$body": "/analyzing/" } + - match: { "$body": "/only\\s+supports\\s+string\\s+arrays/" } + +--- +"mvjoin and nomv on an array of a geo_point field are rejected while analyzing": + - skip: + features: + - headers + - do: + catch: bad_request + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = mvjoin(array(gp), ',') | fields x" + - match: { "$body": "/ExpressionEvaluationException/" } + - match: { "$body": "/analyzing/" } + - match: { "$body": "/only\\s+supports\\s+string\\s+arrays/" } + - do: + catch: bad_request + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = array(gp) | nomv x | fields x" + - match: { "$body": "/ExpressionEvaluationException/" } + - match: { "$body": "/analyzing/" } + - match: { "$body": "/only\\s+supports\\s+string\\s+arrays/" } + +--- +"mvjoin and nomv on an array of an object field are rejected while analyzing": + - skip: + features: + - headers + - do: + catch: bad_request + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = mvjoin(array(o), ',') | fields x" + - match: { "$body": "/ExpressionEvaluationException/" } + - match: { "$body": "/analyzing/" } + - match: { "$body": "/only\\s+supports\\s+string\\s+arrays/" } + - do: + catch: bad_request + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = array(o) | nomv x | fields x" + - match: { "$body": "/ExpressionEvaluationException/" } + - match: { "$body": "/analyzing/" } + - match: { "$body": "/only\\s+supports\\s+string\\s+arrays/" } + +--- +"mvjoin and nomv on an array of a nested field are rejected while analyzing": + - skip: + features: + - headers + - do: + catch: bad_request + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = mvjoin(array(ne), ',') | fields x" + - match: { "$body": "/ExpressionEvaluationException/" } + - match: { "$body": "/analyzing/" } + - match: { "$body": "/only\\s+supports\\s+string\\s+arrays/" } + - do: + catch: bad_request + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = array(ne) | nomv x | fields x" + - match: { "$body": "/ExpressionEvaluationException/" } + - match: { "$body": "/analyzing/" } + - match: { "$body": "/only\\s+supports\\s+string\\s+arrays/" } + +--- +"mvjoin of a keyword array still works": + - skip: + features: + - headers + - do: + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = mvjoin(array(k), ',') | fields x" + - match: {"total": 1} + - match: {"datarows": [["a"]]} + +--- +"nomv of a keyword array still works": + - skip: + features: + - headers + - do: + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = array(k) | nomv x | fields x" + - match: {"total": 1} + - match: {"datarows": [["a"]]} + +--- +"mvjoin of a text array still works": + - skip: + features: + - headers + - do: + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = mvjoin(array(t), ',') | fields x" + - match: {"total": 1} + - match: {"datarows": [["a"]]} + +--- +"mvjoin of a date array still works": + - skip: + features: + - headers + - do: + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = mvjoin(array(d), ',') | fields x" + - match: {"total": 1} + - match: {"datarows": [["2024-01-01 00:00:00"]]} + +--- +"nomv of a date array still works": + - skip: + features: + - headers + - do: + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = array(d) | nomv x | fields x" + - match: {"total": 1} + - match: {"datarows": [["2024-01-01 00:00:00"]]} + +--- +"mvjoin of a mixed array still works": + - skip: + features: + - headers + - do: + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = mvjoin(array('a', l), ',') | fields x" + - match: {"total": 1} + - match: {"datarows": [["a,3"]]} + +--- +"mvjoin of an array cast to string still works": + - skip: + features: + - headers + - do: + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = mvjoin(array(cast(l as string)), ',') | fields x" + - match: {"total": 1} + - match: {"datarows": [["3"]]} + +--- +"nomv of an array cast to string still works": + - skip: + features: + - headers + - do: + headers: + Content-Type: 'application/json' + ppl: + body: + query: "source=mvjoin_non_string_array_5898 | eval x = array(cast(l as string)) | nomv x | fields x" + - match: {"total": 1} + - match: {"datarows": [["3"]]} diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLFunctionTypeTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLFunctionTypeTest.java index 4383acf40e0..f112cb632b1 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLFunctionTypeTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLFunctionTypeTest.java @@ -300,4 +300,40 @@ public void testMvjoinRejectsNonStringValues() { "source=EMP | eval result = mvjoin(42, ',') | fields result | head 1", "MVJOIN function expects {[ARRAY,STRING]|[ARRAY,STRING,STRING]}, but got [INTEGER,STRING]"); } + + // mvjoin (and nomv, which rewrites to mvjoin) should reject arrays whose known element type is + // not a string, during analysis instead of failing at run time. + @Test + public void testMvjoinRejectsNonStringArrayElements() { + final String expectedMessage = "only supports string arrays"; + // SHORT elements + verifyQueryThrowsException( + "source=EMP | eval result = mvjoin(array(EMPNO), ',') | fields result | head 1", + expectedMessage); + // DECIMAL elements + verifyQueryThrowsException( + "source=EMP | eval result = mvjoin(array(SAL), ',') | fields result | head 1", + expectedMessage); + // BOOLEAN elements + verifyQueryThrowsException( + "source=EMP | eval result = mvjoin(array(EMPNO > 1), ',') | fields result | head 1", + expectedMessage); + // nomv rewrites to mvjoin + verifyQueryThrowsException( + "source=EMP | eval arr = array(EMPNO) | nomv arr | fields arr | head 1", expectedMessage); + } + + @Test + public void testMvjoinAcceptsStringLikeArrayElements() { + // VARCHAR elements + assertNotNull( + getRelNode("source=EMP | eval result = mvjoin(array(ENAME), ',') | fields result")); + // Mixed VARCHAR and numeric elements widen to VARCHAR + assertNotNull( + getRelNode("source=EMP | eval result = mvjoin(array(ENAME, EMPNO), ',') | fields result")); + // Empty array defaults to VARCHAR elements + assertNotNull(getRelNode("source=EMP | eval result = mvjoin(array(), ',') | fields result")); + // HIREDATE is intentionally not covered here: in this test schema it is a plain Calcite DATE, + // not a VARCHAR-backed UDT, so array(HIREDATE) is covered by the integration and YAML tests. + } } diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLNoMvTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLNoMvTest.java index d1310ce60dd..0c71ce4df6e 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLNoMvTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLNoMvTest.java @@ -433,7 +433,10 @@ public void testNoMvWithNullableField() { @Test public void testNoMvWithMultipleNullableFields() { - String ppl = "source=EMP | eval arr = array(MGR, COMM) | nomv arr | head 1 | fields EMPNO, arr"; + // MGR and COMM are numeric, and nomv only supports string arrays, so cast them to string. + String ppl = + "source=EMP | eval arr = array(cast(MGR as string), cast(COMM as string)) | nomv arr |" + + " head 1 | fields EMPNO, arr"; RelNode root = getRelNode(ppl); @@ -441,14 +444,17 @@ public void testNoMvWithMultipleNullableFields() { "LogicalProject(EMPNO=[$0], arr=[$8])\n" + " LogicalSort(fetch=[1])\n" + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4]," - + " SAL=[$5], COMM=[$6], DEPTNO=[$7], arr=[COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array($3," - + " $6)), '\n" + + " SAL=[$5], COMM=[$6], DEPTNO=[$7]," + + " arr=[COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array(SAFE_CAST($3), NUMBER_TO_STRING($6)))," + + " '\n" + "'), '':VARCHAR)])\n" + " LogicalTableScan(table=[[scott, EMP]])\n"; verifyLogical(root, expectedLogical); String expectedSparkSql = - "SELECT `EMPNO`, COALESCE(ARRAY_JOIN(ARRAY_COMPACT(ARRAY(`MGR`, `COMM`)), '\n'), '') `arr`" + "SELECT `EMPNO`, COALESCE(ARRAY_JOIN(ARRAY_COMPACT(ARRAY(TRY_CAST(`MGR` AS STRING)," + + " NUMBER_TO_STRING(`COMM`))), '\n" + + "'), '') `arr`" + LS + "FROM `scott`.`EMP`" + LS @@ -456,6 +462,14 @@ public void testNoMvWithMultipleNullableFields() { verifyPPLToSparkSQL(root, expectedSparkSql); } + @Test + public void testNoMvNumericArrayError() { + // nomv only supports string arrays, so an array of a numeric field is rejected at analysis + final String ppl = "source=EMP | eval arr = array(EMPNO) | nomv arr | head 1"; + + verifyQueryThrowsException(ppl, "only supports string arrays"); + } + @Test public void testNoMvWithMixedNullableAndNonNullableFields() { String ppl =