fix: [bug] narrow BIGINT to INTEGER for int-domain function arguments (#5660)

[AjimelecGonzalez]

Description

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 functions whose implementations require int, narrow BIGINT arguments back to INTEGER at the plan layer.

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.

Testing:

• Unit tests in RelJsonSerializerTest covering the serialization-layer type preservation.
• Integration tests in CalciteArrayFunctionIT, CalciteTextFunctionIT, and CalciteMathematicalFunctionIT covering mvindex, left, right, substring, round, truncate, conv, and sha2 with
  arithmetic and cast(x as long) arguments.
• Regression tests confirming arithmetic with doc-value fields, aggregations, cast(x as long), and long-field arithmetic still return the correct BIGINT types.

Related Issues

Resolves #5660

Related to #5603 (introduced the integer arithmetic widening that exposed this)

Check List

• New functionality includes testing.
• New functionality has been documented.
• New functionality has javadoc added.
• New functionality has a user manual doc added.
• New PPL command checklist all confirmed.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff or -s.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 0d3b11a)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ No major issues detected

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 0d3b11a
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add signature parameter bounds validation Add bounds checking before accessing signature parameter types to prevent potential IndexOutOfBoundsException. The method narrows arguments based on positions in INT_PARAM_POSITIONS but doesn't verify these positions are valid for the matched signature's parameter list. core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java [765-775] for (int pos : intPositions) { - if (pos < args.length && args[pos].getType().getSqlTypeName() == SqlTypeName.BIGINT) { + if (pos < args.length && pos < signature.getParamTypes().size() + && 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]); } } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential IndexOutOfBoundsException risk when accessing signature.getParamTypes(). Adding bounds checking (pos < signature.getParamTypes().size()) would prevent runtime errors if INT_PARAM_POSITIONS contains positions beyond the signature's parameter list.	Medium
General	Verify signature expects INTEGER type Validate that the signature's parameter at pos actually expects INTEGER type before narrowing. The current logic assumes all positions in INT_PARAM_POSITIONS always need narrowing, but doesn't verify the signature's expected type matches this assumption. core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java [754-766] 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 (pos < args.length && pos < signature.getParamTypes().size() + && args[pos].getType().getSqlTypeName() == SqlTypeName.BIGINT + && signature.getParamTypes().get(pos).getSqlTypeName() == SqlTypeName.INTEGER) { Suggestion importance[1-10]: 6 __ Why: The suggestion adds validation to ensure the signature's parameter type at pos is actually INTEGER before narrowing. This makes the logic more defensive and explicit, though the current implementation relies on INT_PARAM_POSITIONS being correctly maintained to match signature expectations.	Low

---

Previous suggestions

Suggestions up to commit c1295f6

Category	Suggestion	Impact
Possible issue	Add bounds check for parameter position The loop checks if any argument needs casting but doesn't validate that parameterTypes has enough positions for all arguments. If args.length exceeds the size of any parameter type combination, accessing position i in expectsIntegerAtPosition could cause index issues. Add a bounds check before calling expectsIntegerAtPosition. core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java [771-777] for (int i = 0; i < args.length; i++) { if (args[i].getType().getSqlTypeName() == SqlTypeName.BIGINT + && i < parameterTypes.stream().mapToInt(List::size).min().orElse(0) && expectsIntegerAtPosition(parameterTypes, i)) { needsCast = true; break; } } Suggestion importance[1-10]: 3 __ Why: The suggestion identifies a potential bounds issue, but expectsIntegerAtPosition already handles bounds checking by returning false when position >= combination.size() (line 850-852). The proposed fix adds unnecessary complexity with a stream operation that doesn't improve safety.	Low
General	Verify original types before narrowing The narrowing logic is applied to all non-arithmetic/non-comparison calls, but the method narrowOperandsToOriginalType unconditionally narrows all BIGINT operands to INTEGER. This could break functions that legitimately accept BIGINT parameters. Consider checking if the original call operand types were narrower before applying the cast. opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RexStandardizer.java [92-94] if (!allowNumericTypeWiden) { - standardizedOperands = narrowOperandsToOriginalType(call, standardizedOperands, helper); + standardizedOperands = narrowOperandsToOriginalType(call, standardizedOperands, call.operands, helper); } Suggestion importance[1-10]: 2 __ Why: The suggestion raises a concern about unconditional narrowing, but the improved_code doesn't show how to implement the verification. The current logic is intentional per the PR's design: non-arithmetic/non-comparison functions with BIGINT operands are narrowed because they require int parameters in Calcite runtime methods. The suggestion lacks a concrete implementation.	Low

[github-actions]

Persistent review updated to latest commit 0d3b11a

[dai-chen]

This needs to maintain list of all function that can accept integer?

[dai-chen]

High level question: the root cause is we widen arithmetic expressions to avoid overflow "unconditionally", can we list the options we've explored besides current PR approach?

[dai-chen]

FYI, in case it's useful — I did a quick check of how other databases handle integer arithmetic overflow. There are two independent decisions (partially done in #5603), and they explain our current implementation challenge in Calcite's combination.

Option 1 — what type does INT + INT get?

Option	Engines	INT + INT	consequence
Keep narrow	PostgreSQL, Calcite, Spark, Trino, DuckDB, SQL Server	INT	overflow is possible at 32 bits, so Option 2 applies there
Widen one tier	MySQL, ClickHouse	BIGINT / Int64	overflow unreachable at 32 bits (int32×int32 = 2⁶² < 2⁶³); Option 2 applies at the 64-bit ceiling
One integer width	BigQuery (INT/SMALLINT/BIGINT are aliases for INT64)	INT64	no narrow tier exists; Option 2 applies at 64 bits

Note: widening only works because those engines' function libraries read every integer argument at 64 bits (MySQL val_int() → longlong, ClickHouse getInt, BigQuery INT64-only, Trino @SqlType(INTEGER) long) — unlike Calcite, whose left/right/position/arrayItemOptional/sround/struncate take primitive int with no long overload.

Option 2 — what happens when a value doesn't fit the type it was given?

Option	Engines	observed
Throw	PostgreSQL, DuckDB, Trino, Spark-ANSI, SQL Server, BigQuery, MySQL	PG 2147483647+1 → ERROR 22003 integer out of range (int4pl) · DuckDB → Out of Range Error: Overflow in addition of INT32 · MySQL 9223372036854775807+1 → ERROR 1690 (22003) BIGINT value is out of range
No check — result wraps	Calcite default, Spark legacy, DataFusion, ClickHouse	Calcite 2147483647+1 → -2147483648 · ClickHouse toInt64(9223372036854775807)+1 → -9223372036854775808
Per-call opt-out → NULL	BigQuery SAFE_ADD, Spark try_add, Trino/DuckDB TRY()	explicit escape hatch layered over one of the above — never a default