yashmayya opened a new pull request, #18883: URL: https://github.com/apache/pinot/pull/18883
## Summary In the multi-stage engine, comparing a `TIMESTAMP` column to a sub-second epoch-millis literal (e.g. `WHERE ts = 1761667561482`) silently returned **no rows**. This is a regression surfaced by #18396 and tracked in #18881. The single-stage engine is unaffected (it treats `TIMESTAMP` as a plain `long`). ## Root cause #18396 changed `PinotTypeCoercion#binaryComparisonCoercion` so that for `tsColumn <op> bigintLiteral`, the implicit cast is placed on the literal (`tsColumn <op> CAST(literal AS TIMESTAMP)`) instead of on the column, to keep the column unwrapped for index applicability. The cast target was built via `factory.createSqlType(SqlTypeName.TIMESTAMP)` with no precision. `TypeSystem.getDefaultPrecision(...)` only overrode `DECIMAL`, so `TIMESTAMP` fell through to Calcite's default precision of **0** (whole seconds). When the literal cast is constant-folded, `RexBuilder.clean` rounds the `TimestampString` to the type precision via `TimestampString.round(0)`, truncating the sub-second component: ``` 1761667561482 -> TIMESTAMP '... :01' -> 1761667561000 ``` The stored value is `...561482`; the folded literal is `...561000`, so the predicate never matches. ## Fix Override `TypeSystem.getDefaultPrecision(TIMESTAMP)` to return **3** (millisecond precision), matching Pinot's epoch-millis `LONG` storage. `3` is also Calcite's `MAX_DATETIME_PRECISION`, so it is simultaneously the default and the max for `TIMESTAMP` — no clamping. This keeps the column unwrapped (preserving the #18396 improvement) and preserves milliseconds on the folded literal. The precision is planner-only metadata and is **not** on the wire: the broker→server boundary maps `SqlTypeName.TIMESTAMP` to `ColumnDataType.TIMESTAMP` (backed by `LONG`, no precision), TIMESTAMP literals are serialized to the server as raw epoch-millis, and no execution path reads the SQL precision — so the change is rolling-upgrade safe and does not alter results beyond the intended millisecond preservation. ## Tests - `PinotTypeCoercionTest#testTimestampColumnVsSubSecondLiteralPreservesMillis` — planner-level guard that the folded literal keeps its millis. Fails without the fix (literal truncates to whole seconds). - `TimestampTest#testSubSecondTimestampEqualityQueries` — end-to-end on both engines: a row with a sub-second `TIMESTAMP` is matched by `WHERE tsCol = <epoch-millis>`. On the multi-stage engine this returned 0 rows before the fix (`expected [1] but found [0]`). - Updated `LiteralEvaluationPlans.json` and two `PinotTypeCoercionTest` assertions for the `TIMESTAMP(0)` → `TIMESTAMP(3)` plan rendering (no behavioral change; the folded epoch values are identical for whole-second literals). ## Backward compatibility No wire-format or segment-format change. Affects only multi-stage planner-emitted `TIMESTAMP` types. After upgrade, multi-stage queries comparing a `TIMESTAMP` column to a sub-second epoch-millis literal return the correct rows. Closes #18881 -- 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]
