gortiz opened a new pull request, #19552:
URL: https://github.com/apache/pinot/pull/19552
## Problem
`generateArray` resolves only in the single-stage engine, where
`TransformFunctionFactory` special-cases it by name. The multi-stage engine
offers the typed `generate{Int,Long,Float,Double}Array` scalars instead, but
folding a call to any of them throws:
```
ClassCastException: class [J cannot be cast to class [Ljava.lang.Object;
at
PinotEvaluateLiteralRule.evaluateLiteralOnlyFunction(PinotEvaluateLiteralRule.java:241)
```
`PinotEvaluateLiteralRule` reads an array result by casting it to
`Object[]`, with a special case for `double[]` only, so every other primitive
array fails. `generateDoubleArray` is the only one of the four usable today.
The generators also mishandle their own arguments. A literal call is
evaluated on the broker while planning, so these are broker-side failures:
| Call | Today |
|---|---|
| `generateLongArray(0, 10, 0)` | `ArithmeticException: / by zero` |
| `generateLongArray(10, 0, 1)` | `NegativeArraySizeException: -9` |
| `generateLongArray(0, Long.MAX_VALUE, 1)` | silently empty — the length is
truncated by an `(int)` cast |
| `generateLongArray(0, 1000000000, 1)` | 8 GB allocation, no bound of any
kind |
| `generateLongArray(7, 7, 1)` | rejected, though a zero span is a valid
single element sequence |
## Changes
**`PinotEvaluateLiteralRule`** — read array elements with
`java.lang.reflect.Array`, which handles primitive and object arrays uniformly,
replacing the `Object[]` cast and its `double[]` special case. This fixes
constant folding for any scalar function returning a primitive array, not only
these.
**`GenerateArrayScalarFunction`** (new) — registers `generateArray` for the
multi-stage engine as a `PinotScalarFunction`, which can dispatch on argument
*types*; the method level `@ScalarFunction` annotation dispatches on argument
*count* alone, so one name cannot cover four element types. The element type is
the widest of the arguments:
```sql
generateArray(0, 6, 2) -- ARRAY(0, 2, 4, 6)
generateArray(1633078800000, 1633080600000, 1800000) -- ARRAY(...:BIGINT,
...:BIGINT)
generateArray(0, 1, 0.5) -- ARRAY(0.0E0:DOUBLE,
...)
```
This follows the existing scalar-for-MSE / transform-for-SSE pattern
(`ArrayLengthScalarFunction`). The single-stage transform function is kept,
since it infers the element type from literals in a way the scalar registry
cannot express, but it now delegates to `ArrayFunctions` so the two engines
cannot disagree on the sequence or on which arguments are rejected.
**`ArrayFunctions`** — validate the increment, bound the length at 100 000
elements, add `(start, end)` overloads defaulting the increment to `1` (or `-1`
when counting down), and index each element from `start` instead of
accumulating, so a floating point sequence does not drift. The length is
checked on the step count rather than the length so a span of `Long.MAX_VALUE`
cannot overflow into a small positive size.
## Why
A dense time grid to `UNNEST` and left join a sparse series onto is how gap
filling is expressed relationally in the multi-stage engine. Without a series
generator the grid has to be faked, typically with an unrelated table and
`ROW_NUMBER`, which needs a table with enough rows and hides the bucket count
in a `LIMIT` — get it wrong and the result is silently truncated rather than an
error:
```sql
-- before
WITH someRows AS (SELECT 1 FROM some_other_table LIMIT 6),
timeBuckets AS (SELECT ROW_NUMBER() OVER () * 1800000 + 1633078800000 -
1800000 AS ts FROM someRows)
...
-- after
SELECT ts FROM UNNEST(generateArray(1633078800000, 1633089600000, 1800000))
AS grid(ts)
```
## Backward compatibility
Single-stage behaviour changes in three ways, all of them fixes: a zero span
now yields `[start]` instead of throwing; the two argument form now counts down
when `end < start` instead of erroring; and a bad increment raises
`IllegalArgumentException` rather than `IllegalStateException`, since
validation moved into `ArrayFunctions`. The existing test is updated
accordingly. The length bound is new — a call generating more than 100 000
elements now fails with a clear message instead of allocating.
## Testing
| Suite | Tests |
|---|---|
| `ResourceBasedQueriesTest`, full suite | 3710, 0 failures |
| `QueryCompilationTest` + `UnnestSqlPlannerTest` | 257 |
| `GenerateArrayTransformFunctionTest` + `ArrayFunctionsTest` (pinot-core) |
52 |
| `GenerateArrayFunctionsTest` + `ArrayFunctionsTest` (pinot-common) | 14 |
New coverage:
- `GenerateArrayFunctionsTest` — sequence semantics, every rejected argument
shape, timestamps past 2^53, and the absence of floating point drift.
- `QueryCompilationTest` — the folded array literal, the element type
dispatch, and the planning time rejections.
- `GenerateArray.json` — end to end through `ResourceBasedQueriesTest`,
including the full gap fill query (grid × keys, `LEFT JOIN`, `LAST_VALUE ...
IGNORE NULLS`) returning the carried forward rows.
The new end-to-end cases set `ignoreLiteMode` and `ignoreV2Optimizer`: both
planners reject `UNNEST` outright with `Unexpected relNode type: Uncollect`,
independently of this change.
## Note on `all-functions.yaml`
Only the five affected entries are updated. The snapshot cannot be
regenerated wholesale at the moment: `NotUdf` looks up
`LogicalFunctions.not(boolean)` while the method takes a `Boolean`, so
`ServiceLoader.load(Udf.class)` throws and `UdfTest` cannot run at all. The
file is already stale as a result — 488 entries against 564 registered
functions, with entries such as `and.scalar` still `null`. The five entries
here were rendered with the same mapper the generator uses and verified against
the live registries. The `NotUdf` mismatch looks worth a separate fix.
--
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]
