fix: revert removal of toOffsetDateTime(String timestamp) fixes #Issue 2497#2501
fix: revert removal of toOffsetDateTime(String timestamp) fixes #Issue 2497#2501davecramer merged 4 commits intopgjdbc:masterfrom
Conversation
|
I guess we should restore 3d14ea4#diff-21e3023c84d9d5fb4f5f04678a094fce6ab5af36a5e73fdfaf495646f3b720d5L532 as well. public OffsetDateTime toOffsetDateTime(Time t) { |
| * @param s The ISO formatted date string to parse. | ||
| * @return null if s is null or a OffsetDateTime of the parsed string s. | ||
| * @throws SQLException if there is a problem parsing s. | ||
| * @deprecated in favour of #toOffsetDateTime(String s, boolean adaptToUTC) |
There was a problem hiding this comment.
I think we suggest users to refrain from using TimestampUtils directly. We don't really want them to call toOffsetDateTime(String s, boolean adaptToUTC).
|
It looks like That would allow reverting the signature back to its original state |
… the original function signature of TimestampUtils.toOffsetDateTime
|
let's see if it passes the tests :) |
Well the function needs |
|
Looks fine to me. |
|
|
||
| } | ||
| if ( oid == Oid.TIMETZ ) { | ||
| return getTimestampUtils().toOffsetDateTime(castNonNull(getString(i))); |
There was a problem hiding this comment.
I first thought that we may need the atDate(LOCAL_DATE_EPOCH) here. But its not needed because a ParsedDatestamp instance defaults to epoch, too: https://github.com/davecramer/pgjdbc/blob/28e3434c5f751f54bd49e0a2c2a88dff45524222/pgjdbc/src/main/java/org/postgresql/jdbc/TimestampUtils.java#L165
There was a problem hiding this comment.
are you suggesting a change here or just commenting on the existing code ?
There was a problem hiding this comment.
Hi, this was just a note to the reader (but could possibly added asa comment). The TIMETZ value is just a time in string format. But as above link tells you, the toOffsetDateTime(String) adds the EPOCH already, because the year/month/day values in ParsedTimestamp default to 1970, 1, 1, so when we convert the ParsedDatestamp instance to OffsetDateTime it get the default values. I was just unsure about this when reviewing the patch.
Sorry for the comment, it was just informational.
3 participants
No description provided.