Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,7 @@ public RexNode resolve(
try {
for (Map.Entry<CalciteFuncSignature, FunctionImp> implement : implementList) {
if (implement.getKey().match(functionName.getName(), argTypes)) {
args = narrowBigintArgs(builder, functionName, implement.getKey(), args);
return implement.getValue().resolve(builder, args);
}
}
Expand Down Expand Up @@ -733,6 +734,83 @@ private static boolean containsSubQuery(RexNode node) {
return false;
}

/**
* Narrows BIGINT arguments to INTEGER for function parameters that specifically require INTEGER.
*
* <p>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}.
*
* <p>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.
*
* <p>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).
*
* <p>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<BuiltinFunctionName, int[]> INT_PARAM_POSITIONS =

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}
}
Loading