0dunay0 opened a new pull request, #10601:
URL: https://github.com/apache/paimon/pull/10601
### Purpose` / `### Tests` body that says `Close #<issue>`.
---
## 1. Issue (title and body, to create first)
**Title:** `[Bug] Parquet TIMESTAMP_LTZ(<=3) is written as TIMESTAMP_MILLIS,
which Iceberg readers reject`
**Labels:** bug (applied by the template)
### Search before asking
- [x] I searched in the issues and found nothing similar.
(Related: #8230 and #8238, both open, stale and conflicting. They change
the type for every
TIMESTAMP(<=3) column unconditionally. #8801 and #8797 are the closed
pushdown bug and its fix.)
### Paimon version
master (c9a36a81d7). Also reproduced on 1.4.x.
### Compute Engine
Flink and Spark write the table. Athena (Iceberg reader) fails to read it.
### Minimal reproduce step
1. Create an append table with `'metadata.iceberg.storage' =
'table-location'` (or `hadoop-catalog`)
and a `TIMESTAMP_LTZ(3)` column, for example `ts TIMESTAMP_LTZ(3)`.
2. Write rows with Flink or Spark. Paimon commits Iceberg metadata, which
declares `ts` as `timestamptz`.
3. Query the table through the Iceberg metadata from Athena, Trino, or any
reader that enforces the
Parquet type of the file.
### What doesn't meet your expectations?
The query fails with an error like this (Athena):
```
ICEBERG_BAD_DATA: Field ts's type INT64 in parquet file
s3://.../data-xxx.parquet is incompatible
with type timestamp(6) with time zone defined in table schema
```
Paimon declares the column as Iceberg `timestamptz`, but
`ParquetSchemaConverter` writes
`TIMESTAMP_LTZ(<=3)` as `INT64 (TIMESTAMP(MILLIS, isAdjustedToUTC=true))`.
The Iceberg spec
(v1 and v2) requires `INT64 TIMESTAMP_MICROS` for `timestamptz`. Paimon
itself reads the files fine.
Expected: when Iceberg compatibility is enabled, the Parquet data files use
the unit that Iceberg
requires. Files already written with MILLIS must remain readable, so a table
can hold both.
### Anything else?
Related: #8230 proposes MICROS for all TIMESTAMP(<=3) columns
unconditionally. This changes the
on-disk format for everybody, including tables that never use Iceberg
compatibility.
### Are you willing to submit a PR?
- [x] I'm willing to submit a PR!
---
## 2. PR
**Title:** `[parquet] Write TIMESTAMP_LTZ(<=3) as TIMESTAMP_MICROS when
Iceberg compatibility is enabled`
### Purpose
Linked issue: close #10600
Iceberg requires `INT64 TIMESTAMP_MICROS` for `timestamptz` columns. Paimon
writes
`TIMESTAMP_LTZ(<=3)` as `TIMESTAMP_MILLIS`, so Iceberg readers that enforce
the Parquet type
(Athena, Trino) reject the data files with `ICEBERG_BAD_DATA`, even though
Paimon publishes valid
Iceberg metadata.
This PR:
- Writes `TIMESTAMP_LTZ(<=3)` as `TIMESTAMP_MICROS` when
`metadata.iceberg.storage` is not
`disabled`. The default behavior and `TIMESTAMP` (without time zone) are
unchanged. The option is
read as the literal key because `paimon-format` does not depend on
`paimon-core`
(`IcebergOptions`).
- Makes the read paths follow each file's Parquet timestamp unit, so MILLIS
and MICROS files can be
mixed in one table (for example, files written before the option was
enabled):
- `ParquetSimpleStatsExtractor` decodes footer min/max using the unit in
the file. It used to
choose the unit from the Paimon precision, which is 1000x off for a
MICROS file with precision <= 3.
- `VectorizedColumnReader` no longer lazily decodes the dictionary of an
INT64 timestamp column
whose file unit differs from Paimon's compact timestamp vector (millis
for precision <= 3,
micros otherwise). The raw dictionary was attached without conversion,
which returns values
1000x too large.
- `ParquetRowDataWriter` selects the timestamp writer from the Parquet
schema, and
`TimestampMicrosWriter` accepts precision <= 3.
Unit conversion in `LongTimestampUpdater` and per-file validation of
predicate pushdown
(#8797) are already on master, so this PR does not touch them.
Compatibility notes:
- Readers older than the unit-aware `LongTimestampUpdater` (for example
Paimon 1.4.1) read MICROS
values for a precision <= 3 column 1000x too large, with no error. Upgrade
readers before
enabling the option on a table that older readers still query.
- Turning the option off again is safe. Files stay as they were written, and
readers handle both units.
- Existing MILLIS files are not rewritten. Iceberg readers still fail on
partitions that contain
MILLIS files until those files are rewritten, for example by compaction.
File format: when `metadata.iceberg.storage` is enabled, Parquet files for
`TIMESTAMP_LTZ(<=3)` columns hold `TIMESTAMP_MICROS`. Nothing changes otherwise.
Alternatives considered:
- #8230 writes MICROS for every `TIMESTAMP(<=3)` column unconditionally.
That changes the file
format for all tables and forces every reader to upgrade, so I gated it
behind the Iceberg option.
- A separate Parquet option instead of reading `metadata.iceberg.storage`.
This avoids reading
another module's option key, but a user would have to set two options to
get a working Iceberg
table. Happy to switch if the maintainers prefer it.
### Tests
One new test, `ParquetFormatReadWriteTest#testIcebergTimestampMicros`, with
the option set. It
writes a `TIMESTAMP(3)` and a `TIMESTAMP_LTZ(3)` column, with repeated
values so the columns are
dictionary encoded, and checks:
- the footer: `TIMESTAMP_LTZ(3)` is `MICROS` and `TIMESTAMP(3)` is still
`MILLIS`
- the values are read back unchanged. This fails without the
`VectorizedColumnReader` change.
- a predicate on the MICROS column does not drop the row group that holds
the match
- the footer min/max statistics are decoded correctly. This fails without the
`ParquetSimpleStatsExtractor` change.
`mvn test -pl paimon-format -am
-Dtest='org.apache.paimon.format.parquet.**.*Test'` passes
(318 tests, 1 skipped, which was already skipped). I reverted each of the
two source fixes
separately and confirmed that the test fails on its own assertion.
I also ran the same change as a backport on 1.4.2 against a production
Iceberg-compatible table:
Athena reads the new MICROS files. Its row count and timestamp sum match
Paimon's for the same snapshot.
Athena also reads `TIMESTAMP(3)` columns stored as MILLIS in the same table,
with values identical to the file contents.
--
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]
