Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Null value getting into packed array #4080

Closed
headius opened this issue Aug 16, 2016 · 9 comments
Closed

Null value getting into packed array #4080

headius opened this issue Aug 16, 2016 · 9 comments

Comments

@headius
Copy link
Member

headius commented Aug 16, 2016

As @donv reported on #4020, there's a null getting into activerecord-jdbc. Here's the reproduction steps with Rails 4.2:

rails new -d postgresql rails_pg_npe
cd rails_pg_npe
rake db:create db:migrate

The final db:migrate blows up because of the null.

Here's the trace on my system:

[] ~/projects/jruby/rails_pg_npe $ JRUBY_OPTS=-Xjit.threshold=0 rake db:migrate
[[nil, "schema_migrations"]]
[[nil, "schema_migrations"]]
/Users/headius/projects/jruby/lib/ruby/gems/shared/gems/rake-11.2.2/lib/rake/task.rb:197: warning: singleton on non-persistent Java type Java::JavaLang::NullPointerException (http://wiki.jruby.org/Persistence)
rake aborted!
Java::JavaLang::NullPointerException: 
arjdbc.jdbc.RubyJdbcConnection.isAr42(RubyJdbcConnection.java:3167)
arjdbc.jdbc.RubyJdbcConnection.setStatementParameter(RubyJdbcConnection.java:2172)
arjdbc.jdbc.RubyJdbcConnection.setStatementParameters(RubyJdbcConnection.java:2162)
arjdbc.jdbc.RubyJdbcConnection$8.call(RubyJdbcConnection.java:855)
arjdbc.jdbc.RubyJdbcConnection$8.call(RubyJdbcConnection.java:840)
arjdbc.jdbc.RubyJdbcConnection.withConnection(RubyJdbcConnection.java:3486)
arjdbc.jdbc.RubyJdbcConnection.withConnection(RubyJdbcConnection.java:3467)
arjdbc.jdbc.RubyJdbcConnection.doExecuteQueryRaw(RubyJdbcConnection.java:840)
arjdbc.jdbc.RubyJdbcConnection.executePreparedQueryRaw(RubyJdbcConnection.java:835)
arjdbc.jdbc.RubyJdbcConnection.execute_query_raw(RubyJdbcConnection.java:815)
arjdbc.jdbc.RubyJdbcConnection$INVOKER$i$execute_query_raw.call(RubyJdbcConnection$INVOKER$i$execute_query_raw.gen)
org.jruby.internal.runtime.methods.DynamicMethod.call(DynamicMethod.java:209)
org.jruby.internal.runtime.methods.DynamicMethod.call(DynamicMethod.java:205)
org.jruby.runtime.callsite.CachingCallSite.cacheAndCall(CachingCallSite.java:358)
org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:195)
Users.headius.projects.jruby.lib.ruby.gems.shared.gems.activerecord_minus_jdbc_minus_adapter_minus_1_dot_3_dot_20.lib.arjdbc.postgresql.adapter.invokeOther19:execute_query_raw(/Users/headius/projects/jruby/lib/ruby/gems/shared/gems/activerecord-jdbc-adapter-1.3.20/lib/arjdbc/postgresql/adapter.rb:1340)

And it turns out the reproduction is simple:

def foo(str)
  blah = [[nil, str]]
end
blah = foo("hello")

The inner array constructed in this case is a packed two-element array with the first element null. It should never be null, so something's not right here.

@headius
Copy link
Member Author

headius commented Aug 16, 2016

Ok, the null may be coming via Java integration.

The logic in AR-JDBC treats the incoming nested array as a Java List when retrieving the inner array. If you look at that inner array through the lens of Java integration, a nil value becomes a null value. Perhaps our autocoercion for Java integration is going too deep.

@headius
Copy link
Member Author

headius commented Aug 16, 2016

@kares It looks like you did a bunch of work with ArrayUtils, the utility class we use for doing coercion of Ruby Array to Java. Is it possible your changes are causing it to coerce more deeply than it did before?

@kares
Copy link
Member

kares commented Aug 17, 2016

there's special cases for some array type coercion to speed things up (a lot as array reflection is still slow on Java 8). did switch from RubyArray's entry(int) to eltInternal(int) as I assumed null values shouldn't be in the Array unless something is going wrong - thus things will now "fail fast": ec7e985#diff-2db2df37fad23f3776166284b3228644L142 ... otherwise no changes of coercing "more deeply".

@headius
Copy link
Member Author

headius commented Aug 17, 2016

@kares Ok. I'm still trying to figure out what has changed, though. An array like[[nil, "foo"]] appears to coerce the nil to null, and AR-JDBC is not expecting it.

@headius
Copy link
Member Author

headius commented Aug 17, 2016

@kares FWIW it's easy to reproduce with the instructions about, and we can't release until we figure it out :-)

@headius
Copy link
Member Author

headius commented Aug 17, 2016

Ok @kares is off the hook...I bisected the NPE to some of my packed array changes. Investigating now!

@kares Thanks for taking a look!

@headius
Copy link
Member Author

headius commented Aug 17, 2016

I found the issue. There's a piece of code in arjdbc that does obj.class == RubyArray.class rather than an instanceof check. I do not know why this code was here, but it fails now because there are subclasses of RubyArray as a result of the packed array work (e.g. RubyArrayOneObject, RubyArrayTwoObject).

I have submitted a trivial fix for this in jruby/activerecord-jdbc-adapter#740 and @kares will verify it and cut a release tomorrow.

@donv If you can test my patch now, that would be great. Hopefully you will at least test it after arjdbc release, so we can verify things are working now :-)

@headius headius modified the milestones: Non-Release, JRuby 9.1.3.0 Aug 17, 2016
@headius
Copy link
Member Author

headius commented Aug 17, 2016

This is not a 9.1.3.0 fix so I've moved it to non-release. @kares Should we create another bug in arjdbc to track this or is the PR enough?

@kares
Copy link
Member

kares commented Aug 18, 2016

@headius did the change and released AR-JDBC 1.3.21 to play nicely with jruby-head and next release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants