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

Already on GitHub? Sign in to your account

Fix for uninitialized visitor in jdbc adapter #156

merged 4 commits into from Jan 28, 2012


None yet
5 participants

gfmurphy commented Jan 24, 2012

This changeset initializes the visitor member variable in the jdbc adapter superclass to an instance of the declared visitor class for the adapter name. I think this will fix the issue for all jdbc adapters that was introduced with AR 3.2's visitor accessor located in AbstractAdapter. The AR 3.2 test suite runs with the arjdbc Sqlite3, MySQL and Postgresql adapters with this patch applied.

It should address #132 #155

It also includes a fix for the jdbcsqlite3 adapter dropping precision and scale in AR 3.2 migration tests.


headius commented Jan 24, 2012

Will this make AR-JDBC dependent on Rails 3.2, though? There are still many people running earlier versions. That was the concern that kepy me from making a similar fix originally :(


gfmurphy commented Jan 24, 2012

Good question :-)

It should not make it dependent on AR 3.2 since it only sets an instance variable that is a dependency only in methods defined outside of arjdbc. The interface change in AR was made in the upstream AbstractAdapter.

I have run the arjdbc test suite against AR versions 3.0.9, 3.1.3 and 3.2.0; the tests pass. Both the rails test suites in 3-1-stable and 3-2-stable run relatively cleanly using this change as well. I'm not setup to easily run the rails suite for the 3.0 branch.

So it's not dependent on AR 3.2, but I not completely certain it will run cleanly in AR 3.0.x. My gut says this patch wouldn't break anything.


headius commented Jan 24, 2012

Thank you for the extra testing! I think you're right, and we can go with this fix. We'll do that today.


atambo commented Jan 26, 2012

I'm running into this as well when upgrading to to rails 3.2.

My jdbcmysql_adapter.rb contains only following contains:

require 'arjdbc/mysql'

@nicksieger nicksieger merged commit e9322c6 into jruby:master Jan 28, 2012

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