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 f02db636785..63e0e2b1ade 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 @@ -647,6 +647,7 @@ public RexNode resolve( try { for (Map.Entry implement : implementList) { if (implement.getKey().match(functionName.getName(), argTypes)) { + args = narrowBigintArgs(builder, functionName, implement.getKey(), args); return implement.getValue().resolve(builder, args); } } @@ -733,6 +734,83 @@ private static boolean containsSubQuery(RexNode node) { return false; } + /** + * Narrows BIGINT arguments to INTEGER for function parameters that specifically require INTEGER. + * + *

PPL's integer arithmetic widening (#5603) makes all integer expressions produce BIGINT. + * However, many Calcite runtime methods ({@code SqlFunctions.arrayItemOptional}, {@code left}, + * {@code right}, {@code round}, {@code truncate}, etc.) require Java {@code int} parameters. + * Since {@code SqlTypeFamily.INTEGER} contains BIGINT, the type checker accepts BIGINT arguments + * but code generation fails when the JVM cannot auto-narrow {@code long} to {@code int}. + * + *

This method inspects the matched signature's expected parameter types. If a parameter + * position expects only INTEGER (not BIGINT), and the actual argument is BIGINT, it wraps the + * argument in a CAST to INTEGER. This is safe because the type checker has already validated the + * argument is in the INTEGER family. + * + *

Arithmetic and comparison operators are excluded because they intentionally use BIGINT for + * overflow safety and their implementations accept {@code long}. + */ + private static RexNode[] narrowBigintArgs( + RexBuilder builder, + BuiltinFunctionName functionName, + CalciteFuncSignature signature, + RexNode... args) { + // Only narrow the int-domain control positions of functions known to require Java int params. + int[] intPositions = INT_PARAM_POSITIONS.get(functionName); + if (intPositions == null) { + return args; + } + RexNode[] narrowed = null; + for (int pos : intPositions) { + if (pos < args.length && args[pos].getType().getSqlTypeName() == SqlTypeName.BIGINT) { + if (narrowed == null) { + narrowed = args.clone(); + } + RelDataType intType = + TYPE_FACTORY.createTypeWithNullability( + TYPE_FACTORY.createSqlType(SqlTypeName.INTEGER), args[pos].getType().isNullable()); + narrowed[pos] = builder.makeCast(intType, args[pos]); + } + } + return narrowed != null ? narrowed : args; + } + + /** + * Maps functions whose Calcite runtime implementations have Java {@code int} parameters to the + * specific argument positions that are int-domain "control" parameters (indices, lengths, + * precision, radix, bit-length, mode). + * + *

PPL's integer arithmetic widening (#5603) makes integer expressions produce BIGINT. Since + * {@code SqlTypeFamily.INTEGER} contains BIGINT, calls pass type checking but fail at code + * generation because the JVM cannot auto-narrow {@code long} to {@code int}. We narrow only the + * listed positions, never value/data positions (e.g. {@code ROUND}'s first operand may itself be + * a legitimate BIGINT to round, so only the precision at position 1 is narrowed). + */ + private static final java.util.Map INT_PARAM_POSITIONS = + java.util.Map.ofEntries( + // ITEM(array, index) + java.util.Map.entry(BuiltinFunctionName.INTERNAL_ITEM, new int[] {1}), + // ARRAY_SLICE(array, start, length) + java.util.Map.entry(BuiltinFunctionName.ARRAY_SLICE, new int[] {1, 2}), + // LEFT(string, length) / RIGHT(string, length) + java.util.Map.entry(BuiltinFunctionName.LEFT, new int[] {1}), + java.util.Map.entry(BuiltinFunctionName.RIGHT, new int[] {1}), + // ROUND(numeric, precision) / TRUNCATE(numeric, precision) — narrow precision only + java.util.Map.entry(BuiltinFunctionName.ROUND, new int[] {1}), + java.util.Map.entry(BuiltinFunctionName.TRUNCATE, new int[] {1}), + // SUBSTR(string, start, length) / SUBSTRING(string, start, length) + java.util.Map.entry(BuiltinFunctionName.SUBSTR, new int[] {1, 2}), + java.util.Map.entry(BuiltinFunctionName.SUBSTRING, new int[] {1, 2}), + // RAND(seed) + java.util.Map.entry(BuiltinFunctionName.RAND, new int[] {0}), + // CONV(string, fromBase, toBase) — narrow the two radix args + java.util.Map.entry(BuiltinFunctionName.CONV, new int[] {1, 2}), + // SHA2(string, bitLength) + java.util.Map.entry(BuiltinFunctionName.SHA2, new int[] {1}), + // WEEK(date, mode) + java.util.Map.entry(BuiltinFunctionName.WEEK, new int[] {1})); + /** * Ad-hoc coercion for some functions that require specific casting of arguments. Now it only * applies to the REDUCE function. 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 0f5b2bb5649..a10e27b4c6d 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 @@ -580,6 +580,78 @@ public void testMvindexWithStatsAggregationPushdown() throws IOException { verifySchema(actual, schema("count()", "bigint"), schema("e", "string")); } + @Test + public void testMvindexWithLiteralArithmeticIndex() throws IOException { + // mvindex with user arithmetic as index (1 + 1). PPL widens arithmetic to BIGINT, + // so the ITEM index must be narrowed back to INTEGER at the serialization boundary. + JSONObject actual = + executeQuery( + String.format( + "source=%s | eval arr = array('a', 'b', 'c'), result = mvindex(arr, 1 + 1)" + + " | head 1 | fields result", + TEST_INDEX_BANK)); + + verifySchema(actual, schema("result", "string")); + verifyDataRows(actual, rows("c")); + } + + @Test + public void testMvindexWithEvalDerivedArithmeticIndex() throws IOException { + // mvindex where the index comes from an eval-derived arithmetic value (BIGINT). + JSONObject actual = + executeQuery( + String.format( + "source=%s | eval a = 1 + 1, arr = array('a', 'b', 'c'), result = mvindex(arr, a)" + + " | head 1 | fields result", + TEST_INDEX_BANK)); + + verifySchema(actual, schema("result", "string")); + verifyDataRows(actual, rows("c")); + } + + @Test + public void testMvindexWithFieldDerivedArithmeticIndex() throws IOException { + // mvindex where the index is derived from an integer field via arithmetic (age - N). + JSONObject actual = + executeQuery( + String.format( + "source=%s | where age = 32 | eval arr = array('a', 'b', 'c', 'd', 'e')," + + " result = mvindex(arr, age - 30) | head 1 | fields result", + TEST_INDEX_BANK)); + + verifySchema(actual, schema("result", "string")); + verifyDataRows(actual, rows("c")); + } + + @Test + public void testMvindexRangeWithArithmeticIndices() throws IOException { + // mvindex range access with arithmetic start and end indices (BIGINT). + JSONObject actual = + executeQuery( + String.format( + "source=%s | eval arr = array(1, 2, 3, 4, 5), result = mvindex(arr, 1 + 0, 2 + 1)" + + " | head 1 | fields result", + TEST_INDEX_BANK)); + + verifySchema(actual, schema("result", "array")); + verifyDataRows(actual, rows(List.of(2, 3, 4))); + } + + @Test + public void testMvindexWithCastLongIndex() throws IOException { + // mvindex with an explicit cast(x as long) index. Pre-existing bug: a genuinely-BIGINT value + // handed to ITEM's int parameter must be narrowed to INTEGER. + JSONObject actual = + executeQuery( + String.format( + "source=%s | eval arr = array('a', 'b', 'c'), result = mvindex(arr, cast(1 as" + + " long)) | head 1 | fields result", + TEST_INDEX_BANK)); + + verifySchema(actual, schema("result", "string")); + verifyDataRows(actual, rows("b")); + } + @Test public void testMvfindWithMatch() throws IOException { JSONObject actual = diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteMathematicalFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteMathematicalFunctionIT.java index 0d6bf47f539..857399486fa 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteMathematicalFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteMathematicalFunctionIT.java @@ -5,6 +5,12 @@ package org.opensearch.sql.calcite.remote; +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BANK; +import static org.opensearch.sql.util.MatcherUtils.*; + +import java.io.IOException; +import org.json.JSONObject; +import org.junit.jupiter.api.Test; import org.opensearch.sql.ppl.MathematicalFunctionIT; public class CalciteMathematicalFunctionIT extends MathematicalFunctionIT { @@ -13,4 +19,72 @@ public void init() throws Exception { super.init(); enableCalcite(); } + + @Test + public void testRoundWithArithmeticPrecision() throws IOException { + // ROUND with arithmetic precision argument. PPL arithmetic widens to BIGINT but ROUND + // expects int. + JSONObject actual = + executeQuery( + String.format( + "source=%s | eval result = round(123.456, 1 + 0) | head 1 | fields result", + TEST_INDEX_BANK)); + + verifySchema(actual, schema("result", null, "double")); + verifyDataRows(actual, rows(123.5)); + } + + @Test + public void testTruncateWithArithmeticPrecision() throws IOException { + // TRUNCATE with arithmetic precision argument. PPL arithmetic widens to BIGINT but TRUNCATE + // expects int. + JSONObject actual = + executeQuery( + String.format( + "source=%s | eval result = truncate(123.456, 1 + 0) | head 1 | fields result", + TEST_INDEX_BANK)); + + verifySchema(actual, schema("result", null, "double")); + verifyDataRows(actual, rows(123.4)); + } + + @Test + public void testRoundWithCastLongPrecision() throws IOException { + // ROUND with an explicit cast(x as long) precision. Pre-existing bug: a genuinely-BIGINT + // value handed to ROUND's int parameter must be narrowed to INTEGER. + JSONObject actual = + executeQuery( + String.format( + "source=%s | eval result = round(123.456, cast(1 as long)) | head 1 | fields" + + " result", + TEST_INDEX_BANK)); + + verifySchema(actual, schema("result", null, "double")); + verifyDataRows(actual, rows(123.5)); + } + + @Test + public void testConvWithArithmeticBases() throws IOException { + // CONV with arithmetic base arguments. Both widen to BIGINT but CONV expects int radixes. + JSONObject actual = + executeQuery( + String.format( + "source=%s | eval result = conv('11', 1 + 1, 8 + 2) | head 1 | fields result", + TEST_INDEX_BANK)); + + verifySchema(actual, schema("result", null, "string")); + verifyDataRows(actual, rows("3")); + } + + @Test + public void testSha2WithArithmeticBitLength() throws IOException { + // SHA2 with arithmetic bit-length argument. Widens to BIGINT but SHA2 expects int. + JSONObject actual = + executeQuery( + String.format( + "source=%s | eval result = sha2('abc', 128 + 128) | head 1 | fields result", + TEST_INDEX_BANK)); + + verifySchema(actual, schema("result", null, "string")); + } } diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTextFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTextFunctionIT.java index 765a2caba2c..1699a9cd989 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTextFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTextFunctionIT.java @@ -213,4 +213,69 @@ public void testRegexpMatchInEvalWithConditions() throws IOException { rows("world", false, true), rows("helloworld", true, true)); } + + @Test + public void testLeftWithArithmeticLength() throws IOException { + // LEFT with arithmetic length argument. PPL arithmetic widens to BIGINT but LEFT expects int. + JSONObject actual = + executeQuery( + String.format( + "source=%s | eval result = left(name, 1 + 1) | head 1 | fields result", + TEST_INDEX_STRINGS)); + + verifySchema(actual, schema("result", null, "string")); + } + + @Test + public void testRightWithArithmeticLength() throws IOException { + // RIGHT with arithmetic length argument. PPL arithmetic widens to BIGINT but RIGHT expects int. + JSONObject actual = + executeQuery( + String.format( + "source=%s | eval result = right(name, 1 + 1) | head 1 | fields result", + TEST_INDEX_STRINGS)); + + verifySchema(actual, schema("result", null, "string")); + } + + @Test + public void testSubstringWithArithmeticArgs() throws IOException { + // SUBSTRING with arithmetic start and length. Both widen to BIGINT but SUBSTRING expects int. + JSONObject actual = + executeQuery( + String.format( + "source=%s | eval result = substring(name, 1 + 0, 2 + 1) | head 1 | fields result", + TEST_INDEX_STRINGS)); + + verifySchema(actual, schema("result", null, "string")); + } + + @Test + public void testLeftWithCastLongLength() throws IOException { + // LEFT with an explicit cast(x as long) length. Pre-existing bug: a genuinely-BIGINT value + // handed to LEFT's int parameter must be narrowed to INTEGER. + JSONObject actual = + executeQuery( + String.format( + "source=%s | eval result = left('abcdef', cast(2 as long)) | head 1 | fields" + + " result", + TEST_INDEX_STRINGS)); + + verifySchema(actual, schema("result", null, "string")); + verifyDataRows(actual, rows("ab")); + } + + @Test + public void testRightWithCastLongLength() throws IOException { + // RIGHT with an explicit cast(x as long) length. + JSONObject actual = + executeQuery( + String.format( + "source=%s | eval result = right('abcdef', cast(2 as long)) | head 1 | fields" + + " result", + TEST_INDEX_STRINGS)); + + verifySchema(actual, schema("result", null, "string")); + verifyDataRows(actual, rows("ef")); + } }