daguimu commented on PR #28071: URL: https://github.com/apache/flink/pull/28071#issuecomment-4399355040
@davidradl summary of the new revision (force-pushed to `aae33cc`): **Scope change.** Following up on the discussion above — the writer-only fix would have flipped a symmetric bug into an asymmetric one and broken Flink-internal round-trips with a `timestamp-micros` schema. To avoid that regression I expanded the PR to fix the reader as well: - `RowDataToAvroConverters` — unchanged from the previous revision. Writer honors the schema's logical type when emitting the long value. - `AvroToRowDataConverters.convertToTimestamp` — now takes the Flink type precision and decodes the long as microseconds when `precision > 3`. The mapping precision↔logical-type is the one `AvroSchemaConverter` already uses in both directions, so in normal Flink usage the reader and writer agree without needing to plumb the schema through the read API. - Negative epoch micros are decoded with `Math.floorDiv` / `Math.floorMod` so the nano-of-millis component stays in the `[0, 999_999]` range that `TimestampData.fromEpochMillis(long, int)` requires. **Tests.** A new `AvroToRowDataConvertersTest` covers the reader for `timestamp-micros`, `local-timestamp-micros`, the millis regression case, and a pre-1970 negative-micros case. There are also two end-to-end round-trip tests that drive both converters together — these are the guard against the read/write asymmetry that triggered this discussion in the first place. PR title and description are updated to reflect the bidirectional scope. The `Spotless` and import-collision feedback from your earlier reviews are still addressed in the writer file. Happy to keep iterating if anything looks off. -- 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]
