yashmayya opened a new pull request, #19949:
URL: https://github.com/apache/pinot/pull/19949
This reverts #19790. That PR maps `DECIMAL(p, 0)` with `19 <= p <= 38` to
`BIG_DECIMAL` (was `LONG`) in the multi-stage engine. The change reaches more
queries than planned, and it causes wrong results. #19790 is only on master
(`1.7.0-SNAPSHOT`), so this revert brings back the released behavior.
## Problem
Common expressions get the type `DECIMAL(19, 0)`:
1. Calcite gives a `DECIMAL` type to `MOD(doubleCol, 100)`, `longCol * 1.5`
and `ts / 1000.0`. Pinot's `TypeSystem` caps these derived types at
`DECIMAL(19, 1)`.
2. Calcite gives `CEIL` and `FLOOR` of `DECIMAL(p, s)` the type `DECIMAL(p,
0)`. Here, that is `DECIMAL(19, 0)`.
3. With #19790, `DECIMAL(19, 0)` maps to `BIG_DECIMAL`.
As a result, `FLOOR(ts / 1000.0)`, `FLOOR(x * 0.001)` and
`CEIL(MOD(doubleCol, 100))` return `BIG_DECIMAL` (was `LONG`). The broker sends
these values as strings with a fraction, for example `"4.0"` instead of `4`.
`SUM`, `MIN`, `MAX`, `AVG`, `GROUP BY`, `DISTINCT`, window functions, `UNION`
and `CASE` carry the new type.
Servers compute `ceil` and `floor` in `double`. The value `3.0` becomes
`BigDecimal("3.0")`, but an `INT` column cast to `BIG_DECIMAL` becomes
`BigDecimal("3")`. Hash joins and set operations compare keys with
`BigDecimal.equals`, which also compares the scale. Results for a table with a
row where `d = 3.7` and `intCol = 3`:
| Query | Before #19790 | After #19790 |
|---|---|---|
| `t1 JOIN t2 ON FLOOR(MOD(t1.d, 100)) = t2.intCol` | 1 row | 0 rows |
| `WHERE intCol IN (SELECT FLOOR(MOD(d, 100)) ...)`, physical optimizer | 1
row | 0 rows |
| `WHERE intCol NOT IN (SELECT FLOOR(MOD(d, 100)) ...)` | row with `intCol =
3` excluded | row with `intCol = 3` returned |
| `SELECT FLOOR(MOD(d, 100)) ... INTERSECT SELECT intCol ...` | `3` | empty |
| `SELECT FLOOR(MOD(d, 100)) ... EXCEPT SELECT intCol ...` | `3` removed |
`3.0` kept |
| `SELECT FLOOR(MOD(d, 100)) ... UNION SELECT intCol ...` | `3` once | `3`
and `3.0` |
## Why a revert and not a fix forward
- The bugs that #19790 fixes need an integer literal beyond the `BIGINT`
range, or a `CAST` to `DECIMAL(10..38, 0)`. These are rare. The shapes above
are common.
- A fix forward needs two larger changes. `CEIL` and `FLOOR` need a smaller
precision, but Calcite's `SqlFloorFunction` blocks a replacement through the
operator table. Joins and set operations need `BigDecimal` keys that do not
depend on the scale.
## What comes back with the revert
- The wrap of `DECIMAL(p, 0)` literals beyond the `INT` and `LONG` range, as
listed in #19790. A later PR can fix this at the literal, without a change to
the type mapping for expressions.
- `SUM(bigDecimalCol) IN (1, 2)` and `ARRAY_AGG(intCol, 'LONG')` fail again,
as before #19790. These fixes do not need the new type mapping, so they can
come back as separate PRs.
## Tests
The revert applies to master with no conflicts. These tests pass locally:
`RelToPlanNodeConverterTest`, `RexExpressionUtilsTest`,
`PRelToPlanNodeConverterTest`, `ResourceBasedQueryPlansTest`,
`QueryCompilationTest`, `FilterOperatorTest` and `ResourceBasedQueriesTest`.
--
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]
