yashmayya opened a new pull request, #19948: URL: https://github.com/apache/pinot/pull/19948
When all arguments of a function call are literals, the multi-stage engine (MSE) evaluates the call at planning time (`PinotEvaluateLiteralRule`). The rule passed the values that Calcite stores: `BigDecimal` for INTEGER, BIGINT and DECIMAL literals, and `Long` millis for TIMESTAMP literals. The runtime passes `Integer`, `Long`, `Double` and `Timestamp`. `Object` and varargs parameters get the value as is, and some functions dispatch on its class. So the folded result was different from the runtime result. For example, `toThetaSketch(5)` sketched the string `"5"`, not the number 5. Found while reviewing #19757. ## Changes - `PinotEvaluateLiteralRule` converts each literal as the runtime does: `RexExpressionUtils.fromRexLiteral`, then `ColumnDataType.toExternal`, as in `FunctionOperand`. It converts the literals only after it finds the function, because some operators take literals that are not values (the Sarg of a SEARCH). Literals that the runtime cannot represent, for example INTERVAL, keep the Calcite value, as before. - `inIdSet` is deterministic again. #19757 set `isDeterministic = false` only because folding passed a `BigDecimal` for an INT literal. ## Which functions had the problem | Function | Result | |---|---| | `toThetaSketch`, `toHLL`, `toCpcSketch`, `toULL`, `toIntegerSumTupleSketch` | Yes, for INTEGER, BIGINT and DECIMAL literals. REAL literals already matched. | | `inIdSet` | Yes. Folding failed with "Cannot look up BIG_DECIMAL values". | | `isDistinctFrom`, `isNotDistinctFrom` | Yes (`isDistinctFrom(1.5, 1.50)` was `true`), now fixed by #19791, which compares numbers by value. | | `arrayValueConstructor` | Values were correct. Primitive array results are fixed by #19552. | | `nullIf` | No. `NULLIF` resolves to the Calcite operator, which becomes a CASE. | | `cast` | Not from this rule. Calcite folds `CAST(<literal> AS ...)` first, with SQL-standard text. That is a separate problem. | ## Behavior changes Only the broker folds. These results change, and now match the runtime and the single-stage engine (SSE): - Sketch functions on numeric literals return other bytes, for example `toThetaSketch(5)`. - A DECIMAL literal with a scale arrives as a `Double`, the type that the planner gives it. For example, `jsonFormat(1.50)` is now `'1.5'` (was `'1.50'`), and `bigDecimalToBytes(1.50)` returns other bytes. - A TIMESTAMP literal arrives as a `Timestamp`. Functions with `long` or `Timestamp` parameters do not change. `toThetaSketch(TIMESTAMP '...')` now fails at planning time with the runtime error. Before, it sketched the millis. - Both engines fold `IN_ID_SET` on literals, so a value type mismatch fails at planning time, not at runtime. `inIdSet` is now IMMUTABLE, so ingestion transforms accept it. ## Single-stage engine `CompileTimeFunctionsInvoker` needs no change. The SSE parser already gives literals the Java types of the SSE runtime (`Integer`, `Long`, `Double`, `String`, `Boolean`, `byte[]`). For example, SSE folds `toThetaSketch(5)` with an `Integer`, and gives the same bytes as MSE now. SSE has no TIMESTAMP literal type: the parser turns `TIMESTAMP '...'` into a STRING literal, and the SSE runtime gets the same string. A `Timestamp` at compile time only would make compile time and runtime disagree. ## Tests - `LiteralEvaluationPlans.json`: `IN_ID_SET` on INT and BIGINT literals, BIGINT, REAL, BOOLEAN and TIMESTAMP array literals, integer literals beyond BIGINT, a function on an INTERVAL literal, and a SEARCH on a folded literal. - `LiteralExpressions.json` (runtime, compared with H2, both planners): each sketch function on a literal against the same call on column values. - `IdSet.json`: `IN_ID_SET` on a BIGINT literal. - `IdSetQueriesTest`: SSE folds `IN_ID_SET` on literals. Without the change, the sketch and `IN_ID_SET` tests fail. Without the INTERVAL branch or the late conversion, the INTERVAL and SEARCH plan tests fail. -- 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]
