This is an automated email from the ASF dual-hosted git repository.
yashmayya pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 51b9d74dd7b Widen INT arithmetic to LONG in plus/minus/mult scalar
functions (#19036)
51b9d74dd7b is described below
commit 51b9d74dd7b7e3240c47a0b403d2e3aa146a330a
Author: Yash Mayya <[email protected]>
AuthorDate: Wed Jul 22 14:27:44 2026 -0400
Widen INT arithmetic to LONG in plus/minus/mult scalar functions (#19036)
---
.../scalar/arithmetic/MinusScalarFunction.java | 10 +++--
.../scalar/arithmetic/MultScalarFunction.java | 10 +++--
.../scalar/arithmetic/PlusScalarFunction.java | 10 +++--
.../arithmetic/ArithmeticScalarFunctionTest.java | 43 ++++++++++++++++++++++
.../PostAggregationFunctionTest.java | 17 +++++++++
5 files changed, 81 insertions(+), 9 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/arithmetic/MinusScalarFunction.java
b/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/arithmetic/MinusScalarFunction.java
index 6772a665d5b..53052698fe3 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/arithmetic/MinusScalarFunction.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/arithmetic/MinusScalarFunction.java
@@ -32,9 +32,13 @@ public class MinusScalarFunction extends
BaseBinaryArithmeticScalarFunction {
static {
try {
- TYPE_FUNCTION_INFO_MAP.put(ColumnDataType.LONG,
+ FunctionInfo longMinus =
new FunctionInfo(MinusScalarFunction.class.getMethod("longMinus",
long.class, long.class),
- MinusScalarFunction.class, false));
+ MinusScalarFunction.class, false);
+ // INT has no dedicated overload; widen it to LONG so whole-number
arithmetic stays
+ // integral instead of falling back to DOUBLE.
+ TYPE_FUNCTION_INFO_MAP.put(ColumnDataType.INT, longMinus);
+ TYPE_FUNCTION_INFO_MAP.put(ColumnDataType.LONG, longMinus);
TYPE_FUNCTION_INFO_MAP.put(ColumnDataType.DOUBLE,
new FunctionInfo(MinusScalarFunction.class.getMethod("doubleMinus",
double.class, double.class),
MinusScalarFunction.class, false));
@@ -47,7 +51,7 @@ public class MinusScalarFunction extends
BaseBinaryArithmeticScalarFunction {
protected FunctionInfo functionInfoForType(ColumnDataType argumentType) {
FunctionInfo functionInfo = TYPE_FUNCTION_INFO_MAP.get(argumentType);
- // Fall back to double based comparison by default
+ // Fall back to double based arithmetic by default
return functionInfo != null ? functionInfo :
TYPE_FUNCTION_INFO_MAP.get(ColumnDataType.DOUBLE);
}
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/arithmetic/MultScalarFunction.java
b/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/arithmetic/MultScalarFunction.java
index bf0dacfe809..d22986432c7 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/arithmetic/MultScalarFunction.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/arithmetic/MultScalarFunction.java
@@ -32,9 +32,13 @@ public class MultScalarFunction extends
BaseBinaryArithmeticScalarFunction {
static {
try {
- TYPE_FUNCTION_INFO_MAP.put(ColumnDataType.LONG,
+ FunctionInfo longMult =
new FunctionInfo(MultScalarFunction.class.getMethod("longMult",
long.class, long.class),
- MultScalarFunction.class, false));
+ MultScalarFunction.class, false);
+ // INT has no dedicated overload; widen it to LONG so whole-number
arithmetic stays
+ // integral instead of falling back to DOUBLE.
+ TYPE_FUNCTION_INFO_MAP.put(ColumnDataType.INT, longMult);
+ TYPE_FUNCTION_INFO_MAP.put(ColumnDataType.LONG, longMult);
TYPE_FUNCTION_INFO_MAP.put(ColumnDataType.DOUBLE,
new FunctionInfo(MultScalarFunction.class.getMethod("doubleMult",
double.class, double.class),
MultScalarFunction.class, false));
@@ -47,7 +51,7 @@ public class MultScalarFunction extends
BaseBinaryArithmeticScalarFunction {
protected FunctionInfo functionInfoForType(ColumnDataType argumentType) {
FunctionInfo functionInfo = TYPE_FUNCTION_INFO_MAP.get(argumentType);
- // Fall back to double based comparison by default
+ // Fall back to double based arithmetic by default
return functionInfo != null ? functionInfo :
TYPE_FUNCTION_INFO_MAP.get(ColumnDataType.DOUBLE);
}
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/arithmetic/PlusScalarFunction.java
b/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/arithmetic/PlusScalarFunction.java
index ab16bf590c3..006cff64d4d 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/arithmetic/PlusScalarFunction.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/arithmetic/PlusScalarFunction.java
@@ -32,9 +32,13 @@ public class PlusScalarFunction extends
BaseBinaryArithmeticScalarFunction {
static {
try {
- TYPE_FUNCTION_INFO_MAP.put(ColumnDataType.LONG,
+ FunctionInfo longPlus =
new FunctionInfo(PlusScalarFunction.class.getMethod("longPlus",
long.class, long.class),
- PlusScalarFunction.class, false));
+ PlusScalarFunction.class, false);
+ // INT has no dedicated overload; widen it to LONG so whole-number
arithmetic stays
+ // integral instead of falling back to DOUBLE.
+ TYPE_FUNCTION_INFO_MAP.put(ColumnDataType.INT, longPlus);
+ TYPE_FUNCTION_INFO_MAP.put(ColumnDataType.LONG, longPlus);
TYPE_FUNCTION_INFO_MAP.put(ColumnDataType.DOUBLE,
new FunctionInfo(PlusScalarFunction.class.getMethod("doublePlus",
double.class, double.class),
PlusScalarFunction.class, false));
@@ -47,7 +51,7 @@ public class PlusScalarFunction extends
BaseBinaryArithmeticScalarFunction {
protected FunctionInfo functionInfoForType(ColumnDataType argumentType) {
FunctionInfo functionInfo = TYPE_FUNCTION_INFO_MAP.get(argumentType);
- // Fall back to double based comparison by default
+ // Fall back to double based arithmetic by default
return functionInfo != null ? functionInfo :
TYPE_FUNCTION_INFO_MAP.get(ColumnDataType.DOUBLE);
}
diff --git
a/pinot-common/src/test/java/org/apache/pinot/common/function/scalar/arithmetic/ArithmeticScalarFunctionTest.java
b/pinot-common/src/test/java/org/apache/pinot/common/function/scalar/arithmetic/ArithmeticScalarFunctionTest.java
index 6f9fedef585..313d80e570a 100644
---
a/pinot-common/src/test/java/org/apache/pinot/common/function/scalar/arithmetic/ArithmeticScalarFunctionTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/common/function/scalar/arithmetic/ArithmeticScalarFunctionTest.java
@@ -18,8 +18,13 @@
*/
package org.apache.pinot.common.function.scalar.arithmetic;
+import java.util.List;
+import org.apache.pinot.common.function.FunctionInfo;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
import org.testng.annotations.Test;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertThrows;
@@ -39,4 +44,42 @@ public class ArithmeticScalarFunctionTest {
assertThrows(ArithmeticException.class, () ->
NegateScalarFunction.intNegate(Integer.MIN_VALUE));
assertThrows(ArithmeticException.class, () ->
NegateScalarFunction.longNegate(Long.MIN_VALUE));
}
+
+ /**
+ * Regression test: additive/multiplicative operators expose only LONG and
DOUBLE overloads, so an INT x INT pair
+ * must widen to the LONG overload. Otherwise the equal-argument-types
branch in
+ * {@link BaseBinaryArithmeticScalarFunction} resolves the (absent) INT
overload and silently falls back to DOUBLE,
+ * flipping whole-number arithmetic such as {@code COUNT(DISTINCT ...) * 60}
from LONG to DOUBLE.
+ */
+ @Test
+ public void testWholeNumberArithmeticWidensToLong() {
+ ColumnDataType[] intOperands = {ColumnDataType.INT, ColumnDataType.INT};
+ List<BaseBinaryArithmeticScalarFunction> functions =
+ List.of(new PlusScalarFunction(), new MinusScalarFunction(), new
MultScalarFunction());
+ for (BaseBinaryArithmeticScalarFunction function : functions) {
+ FunctionInfo functionInfo = function.getFunctionInfo(intOperands);
+ assertNotNull(functionInfo, function.getName() + " should resolve an
overload for (INT, INT)");
+ assertEquals(functionInfo.getMethod().getReturnType(), long.class,
+ function.getName() + "(INT, INT) must widen to LONG");
+ }
+ }
+
+ /**
+ * Companion invariant to {@link #testWholeNumberArithmeticWidensToLong()}:
operators that expose a dedicated INT
+ * overload (and cannot overflow for equal-width operands) must keep INT x
INT as INT. This asymmetry is why the
+ * widening fix lives in the individual functions rather than the shared
{@link BaseBinaryArithmeticScalarFunction}
+ * dispatch - a blanket dispatch change would force these to LONG as well.
+ */
+ @Test
+ public void testIntPreservingArithmeticStaysInt() {
+ ColumnDataType[] intOperands = {ColumnDataType.INT, ColumnDataType.INT};
+ List<BaseBinaryArithmeticScalarFunction> functions =
+ List.of(new ModScalarFunction(), new GreatestScalarFunction(), new
LeastScalarFunction());
+ for (BaseBinaryArithmeticScalarFunction function : functions) {
+ FunctionInfo functionInfo = function.getFunctionInfo(intOperands);
+ assertNotNull(functionInfo, function.getName() + " should resolve an
overload for (INT, INT)");
+ assertEquals(functionInfo.getMethod().getReturnType(), int.class,
+ function.getName() + "(INT, INT) should stay INT");
+ }
+ }
}
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/query/postaggregation/PostAggregationFunctionTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/query/postaggregation/PostAggregationFunctionTest.java
index 63e9591f72c..6981b012a1c 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/query/postaggregation/PostAggregationFunctionTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/query/postaggregation/PostAggregationFunctionTest.java
@@ -134,4 +134,21 @@ public class PostAggregationFunctionTest {
// The function returns null if any argument is null
assertNull(function.invoke(new Object[]{null, 1}));
}
+
+ /**
+ * Regression test for the single-stage post-aggregation path: arithmetic on
whole-number operands - for example
+ * {@code COUNT(DISTINCT ...) * 60}, which reaches here as {@code times(INT,
INT)} because DISTINCTCOUNT is typed
+ * INT and a small integer literal is typed INT - must return LONG rather
than silently widening to DOUBLE.
+ */
+ @Test
+ public void testWholeNumberArithmeticReturnsLong() {
+ for (String functionName : new String[]{"plus", "minus", "times"}) {
+ PostAggregationFunction function =
+ new PostAggregationFunction(functionName, new
ColumnDataType[]{ColumnDataType.INT, ColumnDataType.INT});
+ assertEquals(function.getResultType(), ColumnDataType.LONG, functionName
+ "(INT, INT) should return LONG");
+ }
+ // 100000 * 100000 overflows int but fits long; the result must be a Long,
not a Double.
+ assertEquals(new PostAggregationFunction("times", new
ColumnDataType[]{ColumnDataType.INT, ColumnDataType.INT})
+ .invoke(new Object[]{100000, 100000}), 10_000_000_000L);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
