loustler commented on PR #11660: URL: https://github.com/apache/seatunnel/pull/11660#issuecomment-5239424980
@DanielLeens you're right, and I was wrong. Thank you for checking it rather than letting it pass. The `timestamp-millis` vs `local-timestamp-millis` analysis is in my own first comment on this PR, 2026-08-05T11:12:03Z. So is the numbered choice @SEZ9 was answering — I asked "tell me which you prefer" and listed (1) a case in `ParquetWriteStrategyEvolutionTest` or (2) a dedicated logical-type test class. And so is the `JobStateEventTest` CI note. Every part of @SEZ9's comment that I said I couldn't find was responding to something I had written myself. **How I got it wrong matters more than the fact of it.** I swept this thread programmatically and only read the first 180 characters of each comment. The analysis sits at line 24 of a long one, so it never entered my view — and I reported its absence as a finding instead of as the limit of what I had looked at. I have made this exact mistake before in another thread, concluding something wasn't in a log file I had only partially downloaded. Same error, different tool. Concluding absence from a partial read is not a conclusion. I withdraw the paragraph in my 2026-08-09T21:41 comment. @SEZ9, the request was well-founded and I mischaracterised it — apologies. The follow-up issue is now filed as #11743, with the original analysis carried over intact rather than re-derived, since the point of the issue is to preserve reasoning that would otherwise be lost in this thread. One thing I'd like your read on, since it is genuinely open rather than a mistake. @SEZ9 chose option 2 — a dedicated logical-type round-trip test class — and what actually landed in `ff640393c` is a new test method inside the existing `ParquetTypeCoercionTest`, which is option 1's location with option 2's content. Your approving review verified it against the live file and found the coverage sufficient, so I don't want to churn it for its own sake; but it is not what @SEZ9 asked for and I'd rather resolve that explicitly than let an approval paper over it. Happy to extract it into a dedicated class if either of you prefers the original answer honoured. -- 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]
