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, RAND, etc.) take Java int parameters with no long overload. Since SqlTypeFamily.INTEGER contains BIGINT, the call passes type checking but fails at code generation (EnumerableCalc) because the JVM cannot auto-narrow long to int.

Fix:

• When an operator-backed function is resolved, narrow BIGINT arguments back to INTEGER only at operand positions that the operator's own registered type checker declares as a strict INTEGER family. The int-domain positions are derived from the type checker (getParameterTypes()), not a hardcoded per-function list, so new int-parameter functions are handled automatically as long as they register with a family/composite-family type checker.
• A position is narrowed only when INTEGER is the sole numeric type accepted there. Value positions that also accept a wider numeric type are never narrowed — e.g. ROUND's first operand maps to the NUMERIC family ([INTEGER, DOUBLE]), so round(bigint_value, 2) keeps its BIGINT value operand and only
  narrows the precision.
• A few operators that previously registered without an explicit type checker (or with one that did not expose per position families) were given explicit checkers so their INTEGER positions are visible: ARRAY_SLICE, TRUNCATE, and RAND.

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. The narrowing runs at plan time only, short-circuits when no argument is BIGINT, and degrades to a no-op (never fails the query) if an operator's type checker cannot be introspected.

Also fixes the pre-existing case where an explicit cast(x as long) is passed to these functions.

Testing:

• Integration tests in CalciteArrayFunctionIT, CalciteTextFunctionIT, CalciteMathematicalFunctionIT, and CalcitePPLBuiltinFunctionIT covering mvindex, left, right, substring, round, truncate, conv, sha2, and rand with arithmetic and cast(x as long) arguments, across local-execution, pushdown, aggregation, and sort/filter contexts.
• Regression tests confirming arithmetic with doc-value fields, aggregations, cast(x as long), max/sum, and long-field arithmetic still return the correct BIGINT types, and that the no-arg forms of the affected functions (e.g. rand(), truncate(x)) are unchanged.

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 f1957a4)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Overflow Risk Narrowing BIGINT to INTEGER via cast can cause runtime overflow when the BIGINT value exceeds INTEGER range (e.g., a value > 2^31-1). The code does not validate that the BIGINT value fits within INTEGER bounds before casting. If a user passes a large BIGINT (from arithmetic or explicit cast), the narrowed INTEGER may wrap or throw an exception at runtime, leading to incorrect results or query failure. if (args[pos].getType().getSqlTypeName() == SqlTypeName.BIGINT && expectsStrictInteger(parameterTypes, pos)) { 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]); } Exception Swallowing The catch block at line 775 silently swallows IllegalArgumentException and returns the original args without narrowing. If the exception indicates a genuine configuration error (e.g., a malformed type checker), the query proceeds with potentially incorrect types, masking the underlying problem. Consider logging the exception or re-throwing it if it signals an unexpected state rather than a known limitation. } catch (IllegalArgumentException e) { // A type checker whose composition is not purely family-based cannot enumerate per-position // parameter types (see PPLTypeChecker composite implementations). We cannot reason about // int-domain positions, so skip narrowing rather than fail an otherwise-valid query. return args; }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to f1957a4

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add null check for type Add a null check for arg.getType() before calling getSqlTypeName() to prevent potential NullPointerException. The type could be null in edge cases with malformed expressions. core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java [762-768] boolean hasBigint = false; for (RexNode arg : args) { - if (arg.getType().getSqlTypeName() == SqlTypeName.BIGINT) { + if (arg.getType() != null && arg.getType().getSqlTypeName() == SqlTypeName.BIGINT) { hasBigint = true; break; } } Suggestion importance[1-10]: 3 __ Why: While adding a null check for arg.getType() is a defensive programming practice, RexNode.getType() typically never returns null in Calcite's type system. The suggestion addresses a theoretical edge case but is unlikely to prevent actual runtime issues in this context.	Low

---

Previous suggestions

Suggestions up to commit f6d8acb

Category	Suggestion	Impact
Possible issue	Validate INTEGER cast overflow The cast from BIGINT to INTEGER may silently truncate values outside the INTEGER range (-2^31 to 2^31-1), leading to incorrect results. Consider adding overflow validation or documenting this behavior to prevent unexpected data loss. core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java [785-796] for (int pos = 0; pos < args.length; pos++) { if (args[pos].getType().getSqlTypeName() == SqlTypeName.BIGINT && expectsStrictInteger(parameterTypes, pos)) { 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]); + // Note: This cast may truncate values outside INTEGER range + narrowed[pos] = builder.makeCast(intType, args[pos], true); } } Suggestion importance[1-10]: 6 __ Why: The suggestion raises a valid concern about potential overflow when casting from BIGINT to INTEGER. However, the improved_code only adds a comment and changes makeCast to include a true parameter without clear documentation of what this parameter does or how it addresses overflow. The suggestion identifies a legitimate issue but the proposed solution is incomplete and may not actually prevent data loss.	Low

Suggestions up to commit 0d3b11a

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

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]

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

[github-actions]

Persistent review updated to latest commit f6d8acb

[AjimelecGonzalez]

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?

As for the options explored:

Options explored:

1. Revert Widen narrow integer operands in PPL +/-/* to prevent overflow #5603 (don't widen) — reintroduces silent 32-bit overflow. Also doesn't fully fix it: the BIGINT-into-int consumer defect is pre-existing (mvindex(arr, cast(1 as long)) failed before Widen narrow integer operands in PPL +/-/* to prevent overflow #5603).
2. Fix per-function impl (e.g. in MVIndexFunctionImp) — tried in fix: cast mvindex index arithmetic to INTEGER type for Calcite ITEM compatibility (#5660) #5670; the plan-layer cast gets re-derived away, and it doesn't scale to every affected function.
3. Serializer-layer cast (RexStandardizer) — fix: [bug] PPL query with mvindex() fails when plugins.calcite.pushdown.enabled=true (#5660) #5689; fixes pushdown only, misses local EnumerableCalc execution.
4. Conditional widening (only widen when overflow is possible) — can't know operand magnitude at plan time for fields, so you'd widen conservatively anyway — same problem.
5. Narrow at the consumer boundary (this PR) — chosen; single chokepoint, covers all paths, keeps overflow-safe BIGINT arithmetic intact. Positions are derived from each operator's type checker (no hardcoded list appoarch need).

[github-actions]

Persistent review updated to latest commit f1957a4