Bug #11421
Some dates mismatch after being persisted in a table
0%
History
#1 Updated by Artur Școlnic 3 months ago
The issue comes from inconsistent calendar handling during date conversions, not from any specific date. Different parts of the code rely on Java’s GregorianCalendar, which implicitly mixes Julian and Gregorian rules depending on how it’s initialized, leading to different millisecond values for the same logical date and, after persistence, a one-day shift.
A key point here is that Java’s modern date/time model follows the Proleptic Gregorian calendar, which applies Gregorian rules backward in time to keep calculations consistent. This differs from historical reality, where the Julian calendar was used before the Gregorian calendar reform, and the gap between the two systems varies over time. Because our code paths are not consistently aligned with a single calendar system, conversions can yield slightly different results, which then surface as off by a number of days errors.
I suggest we revisit how we use the Calendar, Date, and OffsetDateTime APIs and consider moving to a more consistent approach, or at least ensuring they are used in a uniform way across the codebase.
#3 Updated by Artur Școlnic 3 months ago
- Assignee set to Artur Școlnic
- Status changed from New to WIP
- reviewer Ovidiu Maxiniuc added
#4 Updated by Ovidiu Maxiniuc 3 months ago
- the 1-day difference for 'more recent dates' is, in my current opinion, caused by the date arithmetic involving timezone component while intermediary conversion to datetime(-tz) values. I think this complicates the computations, also affecting performance;
- r16540 (11370b) addressed only the 0-year component of proleptic Gregorian calendar. The date skew need also be fixed. Ancient (pre oct' 1582) dates are incorrectly converted to
OffsetDateTime(the intermediary native date type used low-level inBaseRecord.data[]) and eventually persisted to databases; - date format issue only for negative years (in
date.getIsoDate()) causing to display them as000-100instead of-100.
#5 Updated by Artur Școlnic 3 months ago
Ovidiu, I committed the first round of fixes to 11421a. I still have to address the getIsoDate issue. If you have a few minutes, please look in the branch and confirm that the fixes are right, thanks.
#6 Updated by Ovidiu Maxiniuc 3 months ago
In date.java:4579: the line seems useless. The assignment of time field (setTimeInMillis()) is overwritten a few lines below (at 4588), after millis is slidden to UTC.
datetimetz.java, I think the idea of the algorithm is correct. This can be checked with an inspection of the value in BaseRecord.data[] and query after the record is flushed to SQL. Excepting the formatting issues, we can improve performance:
- apply correction only if the
offsetDaysis 0 and possibly stick to original code if the date is recent (year > 1582). I am not aware how costly are the new time-related operations (I guess they require validations and other non-trivial arithmetic), but usually we will be processing values from latest centuries, and any computation shortcut is useful. In other words, let's spend extra CPU cycles strictly on exceptions, not on normal dates. - the thresholds checks from
julianToGregorianOffsetDays()can be simplified. For example:&& !d.isAfter(LocalDate.of(1582, 10, 14)from the second condition can be dropped since that was the exit condition from previousif.
#7 Updated by Artur Școlnic 3 months ago
Ovidiu Maxiniuc wrote:
In
date.java:4579: the line seems useless. The assignment oftimefield (setTimeInMillis()) is overwritten a few lines below (at 4588), aftermillisis slidden to UTC.
I know it seems like it, but this solved a corner case, the calendar is a mutable, reused variable, something about setting the time prior to the other operations made it behave correctly.
This is the test case it solved
define temp-table tt1 fields dt as date.
DEFINE VARIABLE my-date AS DATE NO-UNDO.
my-date = DATE("02/28/1900").
create tt1.
tt1.dt = my-date.
release tt1.
find first tt1.
message tt1.dt = my-date. // FWD -> false
#8 Updated by Ovidiu Maxiniuc 3 months ago
It prints yes for me in both cases (with and without the extra setTimeInMillis()).
But there is something that does not look fine. Inspecting the tt1 buffer with my IDE the date is, indeed, {java.sql.Date@13275} "1900-02-28". But when expanded, it is odd: it has the time component set to 1:44:24 only to balance it out with +144 timezone:
result = {com.goldencode.p2j.persist.TemporaryBuffer@13189} tt1: 2304 [null, null, null, null, null, null, 1900-02-28]RecordState{CACHED }/1
└ currentRecord = {com.goldencode.work.dmo._temp.Tt1_1_1__Impl__@13190} tt1: 2304 [null, null, null, null, null, null, 1900-02-28]RecordState{CACHED }
└ data = {java.lang.Object[7]@13194}
└ 6 = {java.sql.Date@13275} "1900-02-28"
├ cdate = {sun.util.calendar.Gregorian$Date@14479} "1900-02-28T01:44:24.000+0144"
└ fastTime = -2203977600000
OTOH, evaluating new java.sql.Date(0, 1, 28) results in:
result = {java.sql.Date@15334} "1900-02-28"
├ cdate = {sun.util.calendar.Gregorian$Date@15336} "1900-02-28T00:00:00.000+0144"
└ fastTime = -2203983864000
No time, but the same timezone.