You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "yashmayya (via GitHub)" <gi...@apache.org> on 2026/06/29 18:57:00 UTC
[PR] Preserve millisecond precision for TIMESTAMP comparisons in the multi-stage engine [pinot]
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Preserve millisecond precision for TIMESTAMP comparisons in the multi-stage engine [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #18883:
URL: https://github.com/apache/pinot/pull/18883#issuecomment-4836473325
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/18883?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 57.00%. Comparing base ([`3be6236`](https://app.codecov.io/gh/apache/pinot/commit/3be62366ef5802e4e093b4638f359078dd391d09?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`89570e6`](https://app.codecov.io/gh/apache/pinot/commit/89570e628b062bd56ee063fee68c53b0ba2291ee?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 8 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #18883 +/- ##
=============================================
+ Coverage 37.16% 57.00% +19.83%
+ Complexity 1321 1 -1320
=============================================
Files 3393 2604 -789
Lines 211305 152703 -58602
Branches 33226 24825 -8401
=============================================
+ Hits 78541 87047 +8506
+ Misses 125557 58240 -67317
- Partials 7207 7416 +209
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/18883/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/18883/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/18883/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/18883/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/18883/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/18883/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.00% <100.00%> (+19.83%)` | :arrow_up: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/18883/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.00% <100.00%> (+19.83%)` | :arrow_up: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/18883/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.00% <100.00%> (+19.83%)` | :arrow_up: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/18883/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.00% <100.00%> (?)` | |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/18883/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/pinot/pull/18883?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org