Java::OrgJodaTime::IllegalInstantException on date columns
felipefava opened this issue · 5 comments
I recently updated activerecord 4 to activerecord 5 in a non Rails app with jruby-9.2.7.0. Now we have this version of the gem:
activerecord-jdbcmysql-adapter (52.4-java)
activerecord-jdbc-adapter (= 52.4)
jdbc-mysql (~> 5.1.36, < 9)
I tried also upgrading the version to 52.6 but we still have the issue.
The config ActiveRecord::Base.default_timezone value is :utc.
The first problem we faced, was that datetimes were cast wrongly, but we fixed it with this https://github.com/jruby/activerecord-jdbc-adapter#mysql-specific-notes. More details of that issue here.
However, that fix brings us the following issue.
If we have a record with a Date column where the value of the date is the day where the Daylight Saving Time was made, we have this exception:
Java::OrgJodaTime::IllegalInstantException (Illegal instant due to time zone offset transition (daylight savings time 'gap'): 2019-10-06T00:00:00.000 (America/Asuncion))
The important part of the above exception is this 2019-10-06T00:00:00.000 (America/Asuncion)). To understand better the problem, in the timezone America/Asuncion, on the 2019-10-06 begun the Daylight Saving Time. And if I try to find a record with ActiveRecord that has a column date with the value 2019-10-06 it raises the mentioned exception.
In the code of this gem, I saw that for dates columns initialize a dateTime at the beginning fo the day:
# RubyJbdcConnection.java -> method dateToRuby
return DateTimeUtils.newDateAsTime(context, value, null).callMethod(context, "to_date");
# DateTimeUtils.java -> method newDateAsTime
DateTime dateTime = new DateTime(year, month, day, 0, 0, 0, 0, zone);
return RubyTime.newTime(context.runtime, dateTime, 0);
I don't have experience with java and I don't know if it is necessary, but why it creates a datetime and not a date?
I think it shouldn't fail in this case when what I want to instantiate is a date column and not a datetime.
FYI: with activerecord 4 and activerecord-jdbc-adapter (= 1.3.25) this wasn't an issue
Please let me know if I'm missing something, and thanks for the help.
More details of my problem:
jruby-9.2.7.0 :001 > Bill.find(48129508)
Traceback (most recent call last):
16: from arjdbc.jdbc.RubyJdbcConnection.withConnection(RubyJdbcConnection.java:3514)
15: from arjdbc.jdbc.RubyJdbcConnection$15.call(RubyJdbcConnection.java:1148)
14: from arjdbc.jdbc.RubyJdbcConnection$15.call(RubyJdbcConnection.java:1157)
13: from arjdbc.jdbc.RubyJdbcConnection.mapQueryResult(RubyJdbcConnection.java:1251)
12: from arjdbc.jdbc.RubyJdbcConnection.mapToResult(RubyJdbcConnection.java:2151)
11: from arjdbc.jdbc.RubyJdbcConnection.mapRow(RubyJdbcConnection.java:3718)
10: from arjdbc.mysql.MySQLRubyJdbcConnection.jdbcToRuby(MySQLRubyJdbcConnection.java:184)
9: from arjdbc.jdbc.RubyJdbcConnection.jdbcToRuby(RubyJdbcConnection.java:2189)
8: from arjdbc.jdbc.RubyJdbcConnection.dateToRuby(RubyJdbcConnection.java:2351)
7: from arjdbc.util.DateTimeUtils.newDateAsTime(DateTimeUtils.java:256)
6: from org.joda.time.DateTime.<init>(DateTime.java:503)
5: from org.joda.time.base.BaseDateTime.<init>(BaseDateTime.java:226)
4: from org.joda.time.base.BaseDateTime.<init>(BaseDateTime.java:257)
3: from org.joda.time.chrono.AssembledChronology.getDateTimeMillis(AssembledChronology.java:133)
2: from org.joda.time.chrono.ZonedChronology.getDateTimeMillis(ZonedChronology.java:122)
1: from org.joda.time.chrono.ZonedChronology.localToUTC(ZonedChronology.java:157)
Java::OrgJodaTime::IllegalInstantException (Illegal instant due to time zone offset transition (daylight savings time 'gap'): 2019-10-06T00:00:00.000 (America/Asuncion))
The db record:
mysql> SELECT * FROM bills WHERE id = 48129508 \G;
*************************** 1. row ***************************
id: 48129508
due_date: 2019-10-06
amount: 109281
......
The db table:
mysql> desc bills;
+--------------------------+---------------------+------+-----+---------+----------------+
| Field | Type | Null | Key | Default | Extra |
+--------------------------+---------------------+------+-----+---------+----------------+
| id | bigint(20) unsigned | NO | PRI | NULL | auto_increment |
| due_date | date | YES | | NULL | |
......
Could this change be a possible fix? 🤔
I don't know if this could possibly affect other things or create new bugs, but with this change I don't get the Java::OrgJodaTime::IllegalInstantException error anymore.
Original code
protected IRubyObject dateToRuby(final ThreadContext context,
final Ruby runtime, final ResultSet resultSet, final int column)
throws SQLException {
final Date value = resultSet.getDate(column);
if ( value == null ) {
// FIXME: Do we really need this wasNull check here?
return resultSet.wasNull() ? context.nil : RubyString.newEmptyString(runtime);
}
if ( rawDateTime != null && rawDateTime.booleanValue() ) {
return RubyString.newString(runtime, DateTimeUtils.dateToString(value));
}
return DateTimeUtils.newDateAsTime(context, value, null).callMethod(context, "to_date");
}New code:
protected IRubyObject dateToRuby(final ThreadContext context,
final Ruby runtime, final ResultSet resultSet, final int column)
throws SQLException {
final Date value = resultSet.getDate(column);
if ( value == null ) {
// FIXME: Do we really need this wasNull check here?
return resultSet.wasNull() ? context.nil : RubyString.newEmptyString(runtime);
}
if ( rawDateTime != null && rawDateTime.booleanValue() ) {
return RubyString.newString(runtime, DateTimeUtils.dateToString(value));
}
// This line is the only change
return DateTimeUtils.newDate(context, value);
// Or this also works
return DateTimeUtils.newDateAsTime(context, value, DateTimeZone.UTC).callMethod(context, "to_date");
}Sorry for the delay in responding, I'm catching up now.
If we have a record with a Date column where the value of the date is the day where the Daylight Saving Time was made, we have this exception
Good find!
I don't have experience with java and I don't know if it is necessary, but why it creates a datetime and not a date?
JRuby's implementation of the Ruby Time object uses Joda Time's DateTime class.
Could this change be a possible fix?
It could be! Do you think you could turn your change into a PR (probably using the non-UTC path) and we'll work on it from there?
FWIW I know there's a few methods on DateTime for negotiating these DST boundaries, but I do not currently know the algorithm for properly handling this case.
Sure, I can create the PR in the afternoon, after work.
I think the more similar approach of the current implementation would be this
return DateTimeUtils.newDateAsTime(context, value, DateTimeZone.UTC).callMethod(context, "to_date");In fact, I have currently a fork with that same change using it in production, and it is working fine.
What do you think? Should I go with that fix or the one it uses DateTimeUtils.newDate?