From 0d3b11a8b5ed0394982a3cb51f607306d3469ed0 Mon Sep 17 00:00:00 2001 From: Ajimelec Gonzalez Date: Fri, 28 Aug 2026 15:27:31 -0700 Subject: [PATCH] fix: [bug] narrow BIGINT to INTEGER for int-domain function arguments (#5660) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PPL queries that pass integer arithmetic as an argument to functions requiring Java int parameters fail when Calcite is enabled: mvindex(arr, 1 + 1) -> CompileException: arrayItemOptional(List, long, ...) left('abcdef', 1 + 1) -> Unable to implement: SqlFunctions.left(String, long) round(123.456, 1 + 0) -> SqlFunctions.sround(BigDecimal, long) Root cause: PPL widens INTEGER arithmetic to BIGINT for overflow safety (#5603), so expressions like `1 + 1` produce BIGINT. Many Calcite runtime methods (ITEM, LEFT, RIGHT, ROUND, TRUNCATE, SUBSTRING, CONV, SHA2, etc.) take Java int parameters. Since SqlTypeFamily.INTEGER contains BIGINT, the call passes type checking but fails at code generation because the JVM cannot auto-narrow long to int. The bug surfaces on: - Local execution: Calcite EnumerableCalc codegen -> Unable to implement Fix: - PPLFuncImpTable.resolve: for a known set of functions, narrow BIGINT arguments to INTEGER at the specific int-domain "control" positions (indices, lengths, precision, radix, bit-length, mode) via a per-function position map. Value/data positions are never narrowed, so e.g. round(bigint_value, 2) keeps its BIGINT first operand. Overflow safety is preserved: the arithmetic itself still computes in BIGINT; only the final value handed to an int-domain parameter is narrowed. Arithmetic operators, comparisons, cast(x as long), aggregations, and long-field arithmetic are left untouched. Also fixes the pre-existing case where an explicit cast(x as long) is passed to these functions.
 Issue: https://github.com/opensearch-project/sql/issues/5660 Signed-off-by: Ajimelec Gonzalez --- .../expression/function/PPLFuncImpTable.java | 78 +++++++++++++++++++ .../remote/CalciteArrayFunctionIT.java | 72 +++++++++++++++++ .../remote/CalciteMathematicalFunctionIT.java | 74 ++++++++++++++++++ .../calcite/remote/CalciteTextFunctionIT.java | 65 ++++++++++++++++ 4 files changed, 289 insertions(+) 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")); + } }