On Fri, 10 Jul 2026 12:48:04 GMT, Sholto <[email protected]> wrote:

>> Also covers: [8249280](https://bugs.openjdk.org/browse/JDK-8249280)
>> 
>> I will first give a quick summary of the problem.
>> Put simply, the `LocalDate` form of the `java.sql.Date` is derived using the 
>> `getYear` method of `java.util.Date`. This in turn returns the year of the 
>> normalised internal calendar.
>> However, the internal calendar `getYear` has an extra layer of complexity.
>> The calendar has an additional era field, which captures BC/AD.
>> `getYear` therefore just returns the year _of that era_.
>> For example, the year 6BC and the year 6AD both return `getYear` as 6.
>> 
>> **This means that for BC dates, our `LocalDate` conversion loses the sign of 
>> the year.**
>> 
>> This leads to additional problems down the line, as the year 1BC is for 
>> calculations sake is considered to be year 0 (and 2BC us considered year -1 
>> and so on). As a result, the various leap year calculations are WRONG for 
>> these years, causing year format validation failures in situations like 
>> marshalling/unmarshalling the dates with a DB.
>> 
>> There are two seemingly obvious fixes here, however I will attempt to 
>> explain why I did not proceed with them.
>> 
>> Firstly, it seems sensible is to derive the `LocalDate` from an `Instant` 
>> created from the millisecond representation of the `Date`. After all, why we 
>> are having to use the deprecated `getYear`, `getMonth` and `getDay` methods 
>> anyway?
>> The answer lies in [8061577](https://bugs.openjdk.org/browse/JDK-8061577).
>> The underlying millisecond representation between `java.time.Instant` and 
>> `java.util.Date` is fundamentally different. Read that ticket for a greater 
>> explanation.
>> Ultimately though, it means that the for older dates, the only real way to 
>> bridge between the two calendar systems is to use these year/month/day 
>> methods.
>> 
>> This is where the second possible solution appears.
>> The underlying calendar representation that `java.util.Date` uses actually 
>> does have a year method which gives you the correctly signed year, that 
>> being `getNormalizedYear`.
>> In fact, `java.util.Date` uses the setter counterpart `setNormalizedYear` is 
>> its `setYear` method.
>> Given this, it seems natural that `getYear` should similarly call 
>> `getNormalizedYear`.
>> I think this would be my ideal solution, however I recognise that `get`Year 
>> only returning a positive year is very long standing behaviour. Given how 
>> widely spread `java.util.Date` is, I felt it was perhaps better not to rock 
>> the boat too much.
>> 
>> I have therefore taken the decision to add an ...
>
> Sholto has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   8272194: introduce faster code path for LocalDate/LocalDateTime conversion

> We could potentially expand the number of dates covered by this faster 
> codepath by using a negative unix time representing a much earlier year.

I think we can push the check *close to* the BC/AD switch, as we should account 
for local-time offset.

src/java.sql/share/classes/java/sql/Date.java line 323:

> 321:             // As there is no year 0 in the proleptic Gregorian calendar
> 322:             // we also have to adjust the BC year by 1.
> 323:             // 1 BC becomes year 0, 2 BC becomes year -1 and so on.

I think you meant `GregorianCalendar` rather than "proleptic Gregorian 
calendar" here? `GregorianCalendar` has no year zero, whereas the ISO 
proleptic-year numbering used by `LocalDate` does. Also, there is a typo: 
"Ajdust" should be "Adjust".

src/java.sql/share/classes/java/sql/Date.java line 328:

> 326:             year = calendar.get(Calendar.YEAR);
> 327:         }
> 328:         return LocalDate.of(year, calendar.get(Calendar.MONTH) + 1, 
> calendar.get(Calendar.DAY_OF_MONTH));

The values of these fields should continue to be obtained by calling 
`getYear()`, `getMonth()`, and `getDate()`, rather than from the calendar 
instance. Otherwise, if a subclass overrides any of these methods, the override 
would be ignored.

-------------

PR Review: https://git.openjdk.org/jdk/pull/31808#pullrequestreview-4673806472
PR Review Comment: https://git.openjdk.org/jdk/pull/31808#discussion_r3560965214
PR Review Comment: https://git.openjdk.org/jdk/pull/31808#discussion_r3561017188

Reply via email to