yashmayya opened a new pull request, #19036:
URL: https://github.com/apache/pinot/pull/19036
## Problem
In the single-stage engine's post-aggregation path, arithmetic on two INT
operands returns
DOUBLE instead of LONG. For example `COUNT(DISTINCT ts) * 60` — where
DISTINCTCOUNT is typed
INT and the small integer literal 60 is typed INT — comes back as DOUBLE,
while the multi-stage
engine returns LONG. Prior to #18171 the single-stage result was LONG, so
this is a regression
(it breaks clients that rely on the column type, e.g. object mappers
expecting a whole-number
field).
## Root cause
`MultScalarFunction`, `PlusScalarFunction`, and `MinusScalarFunction`
register only LONG and
DOUBLE overloads and fall back to DOUBLE for any other type. #18171 added an
equal-argument-types
fast-path to `BaseBinaryArithmeticScalarFunction.functionInfoForTypes` that
is evaluated *before*
the "both whole numbers → LONG" rule:
```java
if (argumentType1 == argumentType2) { // INT == INT taken first
return functionInfoForType(argumentType1); // asks for an INT overload...
}
if (argumentType1.isWholeNumber() && argumentType2.isWholeNumber()) {
return functionInfoForType(ColumnDataType.LONG);
}
```
For `(INT, INT)` this calls `functionInfoForType(INT)`; with no INT overload
it falls back to the
DOUBLE overload, so the result is typed DOUBLE. Operators that register an
INT overload (`mod`,
`greatest`, `least`, `moduloOrZero`, `positiveModulo`) are unaffected and
correctly keep INT.
## Fix
Map INT to the existing `long` overload in `plus`/`minus`/`mult`, so `INT x
INT` widens to LONG
(overflow-safe, and consistent with the existing `INT x LONG -> LONG` path
and the multi-stage
engine). The fix is per-function rather than in the shared dispatch, because
`mod`/`greatest`/
`least` legitimately keep `INT x INT -> INT`.
## Tests
- `ArithmeticScalarFunctionTest`: `plus`/`minus`/`mult` `(INT, INT)` resolve
to a `long` overload;
a companion test pins `mod`/`greatest`/`least` `(INT, INT) -> int`.
- `PostAggregationFunctionTest`: `plus`/`minus`/`times` `(INT, INT)` return
LONG, and
`100000 * 100000` comes back as `10000000000L` (proving no int truncation
/ no double widening).
## Follow-up (out of scope)
`plus`/`minus`/`mult` also lack FLOAT and BIG_DECIMAL overloads, so
BIG_DECIMAL operands widen to
DOUBLE (precision loss). This is pre-existing and separate from this
regression.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
