fix: [bug] PPL query with mvindex() fails when plugins.calcite.pushdown.enabled=true (#5660)

[AjimelecGonzalez]

Description

Fix Calcite pushdown CompileException where arithmetic in array index expressions (e.g., mvindex(entity, 1)) produces long instead of int at runtime.

RexStandardizer widens arithmetic operands to BIGINT for doc-value compatibility, but PLUS nodes do not have their type serialized in JSON (unlike CAST/MINUS). On deserialization the type is re-derived from BIGINT operands, breaking operators like ITEM that expect int.

Fix: Wrap arithmetic results in an explicit CAST(INTEGER) in RexStandardizer.visitCall() when the original type is narrower than BIGINT. CAST nodes always serialize their type, so INTEGER is preserved through serialization and deserialization.

Test Added:

• testArithmeticInItemIndexPreservesIntegerType: PLUS(int, int) preserves INTEGER type through serialization round-trip (the main bug fix)
• testArithmeticBigintNotWrappedInCast: PLUS(bigint, bigint) stays BIGINT without unnecessary CAST (no regression)
• testArithmeticWithFieldPreservesIntegerType: field_ref + 1 where field is INTEGER preserves INTEGER (field-based index scenario)
• testMvindexWithNonZeroIndexPushdown: mvindex(arr, 2) works, non-zero literal index with pushdown
• testMvindexWithStatsAggregationPushdown: mvindex(arr, 1) inside stats count() by e

Related Issues

Resolves #5660
See also: #5670 (alternative plan-layer fix that is ineffective because the serialization layer overrides it)

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 83b60f2)

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 Missing return statement The code removes the return statement from the try block but adds it after the if-else logic. However, if allowNumericTypeWiden is false, the method reaches the end without returning a value. This will cause a compilation error. The return statement should be unconditional, not dependent on the if condition. RexNode result = helper.rexBuilder.makeCall(call.getType(), normalized.left, standardizedOperands); if (allowNumericTypeWiden && SqlTypeUtil.isExactNumeric(call.getType()) && !call.getType().getSqlTypeName().equals(SqlTypeName.BIGINT)) { RelDataType targetType = OpenSearchTypeFactory.TYPE_FACTORY.createTypeWithNullability( call.getType(), call.getType().isNullable()); result = helper.rexBuilder.makeCast(targetType, result); } return result;

[github-actions]

Persistent review updated to latest commit ee33606

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 83b60f2

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Remove redundant type creation in cast The cast operation creates a targetType that is identical to call.getType() (same type and nullability), making the cast redundant. This appears to be a logic error - the cast should either convert to a different type or be removed entirely if no type conversion is needed. opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RexStandardizer.java [89-96] if (allowNumericTypeWiden && SqlTypeUtil.isExactNumeric(call.getType()) && !call.getType().getSqlTypeName().equals(SqlTypeName.BIGINT)) { - RelDataType targetType = - OpenSearchTypeFactory.TYPE_FACTORY.createTypeWithNullability( - call.getType(), call.getType().isNullable()); - result = helper.rexBuilder.makeCast(targetType, result); + // Cast is needed to preserve original type after operand widening + result = helper.rexBuilder.makeCast(call.getType(), result); } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that creating targetType with the same type and nullability as call.getType() is redundant. The improved code simplifies this by directly using call.getType() in the cast, which is cleaner and achieves the same result. However, the cast itself is intentional (to preserve type after operand widening), so this is a code quality improvement rather than a critical bug fix.	Medium

---

Previous suggestions

Suggestions up to commit a114262

Category	Suggestion	Impact
Possible issue	Use result's nullability for cast The targetType is created from call.getType() but should be created from result.getType() after standardization. The standardized result may have a different type (e.g., BIGINT) than the original call, and the cast should preserve the original call type, not recreate it from itself. opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RexStandardizer.java [89-96] if (allowNumericTypeWiden && SqlTypeUtil.isExactNumeric(call.getType()) && !call.getType().getSqlTypeName().equals(SqlTypeName.BIGINT)) { RelDataType targetType = OpenSearchTypeFactory.TYPE_FACTORY.createTypeWithNullability( - call.getType(), call.getType().isNullable()); + call.getType(), result.getType().isNullable()); result = helper.rexBuilder.makeCast(targetType, result); } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that targetType should use result.getType().isNullable() instead of call.getType().isNullable() since the standardized result may have different nullability characteristics. This ensures the cast preserves the correct nullability from the standardized expression rather than the original call.	Medium

Suggestions up to commit 3cca658

Category	Suggestion	Impact
General	Remove redundant type creation The targetType is created from call.getType() with the same nullability, making the cast redundant. Consider using call.getType() directly in makeCast() to avoid unnecessary type creation, or verify if a different type transformation is intended. opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RexStandardizer.java [89-96] if (allowNumericTypeWiden && SqlTypeUtil.isExactNumeric(call.getType()) && !call.getType().getSqlTypeName().equals(SqlTypeName.BIGINT)) { - RelDataType targetType = - OpenSearchTypeFactory.TYPE_FACTORY.createTypeWithNullability( - call.getType(), call.getType().isNullable()); - result = helper.rexBuilder.makeCast(targetType, result); + result = helper.rexBuilder.makeCast(call.getType(), result); } Suggestion importance[1-10]: 4 __ Why: The suggestion correctly identifies that targetType is created from call.getType() with the same nullability, making the intermediate variable potentially redundant. However, the cast itself serves a purpose in the logic (preserving integer types after widening), so this is a minor code simplification rather than a functional improvement.	Low

Suggestions up to commit ee33606

Category	Suggestion	Impact
Possible issue	Redundant cast to same type The cast operation creates a targetType that is identical to call.getType() with the same nullability, making the cast redundant. Consider either removing the cast entirely or defining a different target type (e.g., BIGINT) if type widening is the intended behavior. opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RexStandardizer.java [89-96] if (allowNumericTypeWiden && SqlTypeUtil.isExactNumeric(call.getType()) && !call.getType().getSqlTypeName().equals(SqlTypeName.BIGINT)) { RelDataType targetType = - OpenSearchTypeFactory.TYPE_FACTORY.createTypeWithNullability( - call.getType(), call.getType().isNullable()); + OpenSearchTypeFactory.TYPE_FACTORY.createSqlType(SqlTypeName.BIGINT); result = helper.rexBuilder.makeCast(targetType, result); } Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical logic error where targetType is created with the same type as call.getType(), making the cast operation redundant and defeating the purpose of type widening. The improved code properly widens to BIGINT type, which aligns with the condition checking that the type is not already BIGINT.	High

[dai-chen]

Any unit or integration test to cover?

[RyanL1997]

+1 to Chen's comment - lets add more testing coverage on the change

[github-actions]

Persistent review updated to latest commit 3cca658

[github-actions]

Persistent review updated to latest commit a114262

[dai-chen]

1. The changes in this visitCall may have broader impact. Just wonder is it possible to narrow down and only apply the fix for mvindex (ITEM)? Or we confirm this bug happens for any function?
2. Also please add test in CalciteArrayFunctionIT to verify.

[AjimelecGonzalez]

To narrow down the fix for just mvindex or (ITEM) would not work, it has been tried in #5670 where the fix is done in the plan-layer where it the CAST added in the MVIndexFunctionImp does nothing during serialzation. This fix only wraps in CAST when the call is BINARY_ARITHMETIC, exact-numeric, and narrower than BIGINT. Comparisons (return BOOLEAN) and already-BIGINT arithmetic are unaffected. Added integration tests in CalciteArrayFunctionIT to verify.

[github-actions]

Persistent review updated to latest commit 60b50e1

[github-actions]

Persistent review updated to latest commit 83b60f2