You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "q8webmaster (via GitHub)" <gi...@apache.org> on 2026/06/15 00:24:27 UTC
[PR] [parquet] Fix TIMESTAMP(n<=3) reader decoding epoch-microseconds as epoch-milliseconds [paimon]
q8webmaster opened a new pull request, #8238:
URL: https://github.com/apache/paimon/pull/8238
## Problem
After PR #8230 is applied, `TIMESTAMP(n <= 3)` columns are stored as epoch-microseconds under a `MICROS` Parquet annotation. The reader in `ParquetTimestampVector.getTimestamp()` for `precision <= 3` still calls `Timestamp.fromEpochMillis()`, treating the stored epoch-microsecond value as epoch-milliseconds — 1000× too large.
The practical consequences are:
- Any query that reads a low-precision timestamp column returns a year around 58xxx instead of the correct value.
- For values near `Long.MAX_VALUE / 1000`, the multiplication inside `Timestamp.toMicros()` overflows a Java long, producing `ArithmeticException: Millis overflow: <negative long>`.
Both break all readers that consume these columns — Paimon's own vectorised reader, paimon-trino, paimon-spark, and the compact operator (which reads sequence fields such as `op_ts`).
## Root cause
The `precision <= 3` and `precision <= 6` branches in `getTimestamp()` both decode INT64 values via `LongColumnVector`, but they use different factory methods:
| Branch | Method | Correct for |
|---|---|---|
| `precision <= 3` | `Timestamp.fromEpochMillis` | MILLIS-annotated files (pre-#8230) |
| `precision <= 6` | `Timestamp.fromMicros` | MICROS-annotated files ✅ |
After #8230, `precision <= 3` files carry a `MICROS` annotation and store epoch-microseconds — the same encoding as `precision <= 6`. The reader was never updated to match.
## Fix
Change the `precision <= 3` branch to call `Timestamp.fromMicros()`, making it consistent with the `precision <= 6` branch:
```java
if (precision <= 3 && vector instanceof LongColumnVector) {
- return Timestamp.fromEpochMillis(((LongColumnVector) vector).getLong(i));
+ return Timestamp.fromMicros(((LongColumnVector) vector).getLong(i));
```
## Prior art
PR #8230 fixed the writer path: `TIMESTAMP(n <= 3)` now emits a `MICROS` annotation and stores epoch-microseconds. This PR is the companion reader fix.
PR #8231 fixed the same `fromEpochMillis` / `fromMicros` inconsistency in `IcebergConversions.timestampFromBytes` for precision-3 manifest bounds.
## Changes
- `ParquetTimestampVector.java`: `precision <= 3` branch calls `fromMicros()` instead of `fromEpochMillis()`
- `ParquetTimestampVectorTest.java`: three unit tests — low-precision MICROS decoding, mid-precision MICROS decoding (regression), and a test that documents the wrong-year result the bug produces
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [parquet] Add regression test for TIMESTAMP(n<=3) reading MICROS-annotated INT64 files [paimon]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8238:
URL: https://github.com/apache/paimon/pull/8238#issuecomment-5769456480
This pull request has had no activity for 90 days and is being marked stale. If you'd like to keep it open, please leave a comment or push a new commit. Thank you for your contribution to Apache Paimon!
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [parquet] Fix TIMESTAMP(n<=3) reader decoding epoch-microseconds as epoch-milliseconds [paimon]
Posted by "q8webmaster (via GitHub)" <gi...@apache.org>.
q8webmaster closed pull request #8238: [parquet] Fix TIMESTAMP(n<=3) reader decoding epoch-microseconds as epoch-milliseconds
URL: https://github.com/apache/paimon/pull/8238
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [parquet] Add regression test for TIMESTAMP(n<=3) reading MICROS-annotated INT64 files [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #8238:
URL: https://github.com/apache/paimon/pull/8238#discussion_r3457463895
##########
paimon-format/src/main/java/org/apache/parquet/filter2/predicate/ParquetFilters.java:
##########
@@ -299,11 +299,7 @@ private static Comparable<?> toParquetObject(
} else if (value instanceof Timestamp) {
Timestamp timestamp = (Timestamp) value;
int precision = getTimestampPrecision(type);
- if (precision <= 3) {
- // milliseconds
- return timestamp.getMillisecond();
- } else if (precision <= 6) {
- // microseconds
+ if (precision <= 6) {
return timestamp.toMicros();
Review Comment:
This predicate literal is now always epoch micros for TIMESTAMP(0..6), but precision 0..3 files written by existing Paimon versions store epoch milliseconds. The filter is created in ParquetFileFormat before ParquetReaderFactory opens each file, so it cannot see whether a particular file schema is TIMESTAMP_MILLIS or TIMESTAMP_MICROS. For old files, ts = 2024-01-01 becomes 1704067200000000 while the row-group stats/data are around 1704067200000, and Parquet can incorrectly drop matching row groups. We should either build the timestamp filter after reading the file schema or disable/avoid this pushdown for legacy low-precision timestamp files.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [parquet] Fix TIMESTAMP(n<=3) reader decoding epoch-microseconds as epoch-milliseconds [paimon]
Posted by "q8webmaster (via GitHub)" <gi...@apache.org>.
q8webmaster commented on PR #8238:
URL: https://github.com/apache/paimon/pull/8238#issuecomment-4703650995
Closing — the fix is in the wrong layer for master. On master, `LongTimestampUpdater.longTimestamp()` already normalises MICROS-annotated INT64 values to epoch-milliseconds before storing them in the `LongColumnVector`, so `ParquetTimestampVector.getTimestamp()` always receives epoch-ms for precision≤3 and `fromEpochMillis` is correct. The E2E failures confirmed this. The reader bug exists in paimon 1.4.1 where `LongTimestampUpdater` lacks `timestampUnit()` and stores raw file values — but that version is not the right target for this 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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [parquet] Add regression test for TIMESTAMP(n<=3) reading MICROS-annotated INT64 files [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #8238:
URL: https://github.com/apache/paimon/pull/8238#discussion_r3457462618
##########
paimon-format/src/main/java/org/apache/paimon/format/parquet/ParquetSimpleStatsExtractor.java:
##########
@@ -223,8 +223,8 @@ private SimpleColStats toTimestampStats(Statistics<?> stats, int precision) {
if (precision <= 3) {
LongStatistics longStats = (LongStatistics) stats;
return new SimpleColStats(
- Timestamp.fromEpochMillis(longStats.getMin()),
- Timestamp.fromEpochMillis(longStats.getMax()),
+ Timestamp.fromMicros(longStats.getMin()),
Review Comment:
After this change the extractor decodes TIMESTAMP(0..3) footer stats as micros solely from the Paimon field precision. Existing Paimon Parquet files written before this PR use TIMESTAMP_MILLIS and store min/max in epoch milliseconds, so extracting stats for those files (for example during migrate/clone or any metadata regeneration) would turn 2024-01-01T00:00:00.123 into a 1970 timestamp and write incorrect file stats. Can we derive the unit from stats.type().getLogicalTypeAnnotation() / the column metadata, like the reader does, and keep MILLIS for legacy files?
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org