jruby / activerecord-jdbcsqlserver-adapter

SQL Server Adapter for Rails (JRuby JDBC support)
https://github.com/jruby/activerecord-jdbc-adapter/
MIT License
2 stars 4 forks source link

Skip connection.setSendTimeAsDatetime when not available. #4

Open HarlemSquirrel opened 4 years ago

HarlemSquirrel commented 4 years ago

Was getting the following with Rails 5.0 and 5.1

NoMethodError (undefined method `setSendTimeAsDatetime' for #<Java::ComMicrosoftSqlserverJdbc::SQLServerConnection:0x70ee1963>)
Did you mean?  sendTimeAsDatetime

Running SELECT @@VERSION against the db server returned Microsoft SQL Server 2016 (SP2-CU7-GDR) (KB4505222) - 13.0.5366.0 (X64)

This guard clause fixed it for me.

rdubya commented 4 years ago

Can you create a test that shows this failure? Based on this test run: https://travis-ci.org/github/jruby/activerecord-jdbcsqlserver-adapter/jobs/712951989 it doesn't look like this change is needed.

HarlemSquirrel commented 4 years ago

What version of mssql is in metaskills/mssql-server-linux-rails image?

rdubya commented 4 years ago

It looks like it is based on microsoft's image that is tagged as 2017. I don't think that should matter though since the issue is with the driver not having a defined method. Maybe they are doing something interesting in their driver, but I'm not sure why the db version would determine whether a method was defined or not. Can you see what version of the jdbc-mssql gem you have installed?

rdubya commented 4 years ago

Another question, which version of JRuby are you using and how are you including the library? Wondering if maybe something like that is making it so it doesn't exist. From the mssql documentation, it seems like that method should still be there.

HarlemSquirrel commented 4 years ago

jdbc-mssql (0.8.0-java) jruby 9.2.12.0

rdubya commented 4 years ago

That should be basically the same as the tests. The main thing I'm concerned about is whether this change is going to break date handling for other people if they run into a similar issue. I'd like to see some form of broken test to know that it is a gem issue and not something on your system specifically, or some combination of things that needs to have a different fix for the datetime handling.

HarlemSquirrel commented 4 years ago

Is there a Docker container for mssql 2016?

rdubya commented 4 years ago

It doesn't look like it.

HarlemSquirrel commented 4 years ago

What version of Java are these specs running on? I see three three different drivers in https://github.com/JesseChavez/jdbc-mssql/tree/v0.8.0/lib

rdubya commented 4 years ago

It looks like its using Java 1.8 and JRuby 9.2.7.0

iaddict commented 3 years ago

I think I've found the root cause for this issue. I have a project which used the SQLServer adapter for AR 3.2. To make this old adapter work, I manually required an old jdbc driver (sqljdbc4-4.0.2206.jar). When updating to this AR 5.1 adapter I overlooked this and thus it yielded the exact same error. After removing this outdated jdbc driver everything worked flawlessly.

Once the connection is working, you can check the jdbc driver version:

ActiveRecord::Base.connection.jdbc_connection.meta_data.driver_name
# => "Microsoft JDBC Driver 8.4 for SQL Server"
ActiveRecord::Base.connection.jdbc_connection.meta_data.driver_version
# => "8.4.1.0"
rdubya commented 3 years ago

@iaddict interesting, thanks for the info. I wonder if there is any way to check the driver version being used when the gem is loaded so we can throw an exception or something to prevent it from running with an unsupported version.

iaddict commented 3 years ago

@rdubya I just tried to get the version of the jdbc driver and actually it is quite easy.

Java::ComMicrosoftSqlserverJdbc::SQLServerDriver.new.major_version
# => 8

If you like, I can make a pull request that checks the major version right after the require of the mssql jdbc driver.

rdubya commented 3 years ago

Awesome. Yeah that would be great if you have the time. If you want to do it off of 5-2-stable-jdbc, I'll take care of getting it into the other branches.

iaddict commented 3 years ago

@rdubya Just one more thought. Maybe it it would be better to check if a jdbc driver is already loaded in the jdbc-mssql gem (https://github.com/JesseChavez/jdbc-mssql/blob/master/lib/jdbc/mssql.rb). Or we check if a driver is already loaded, warn about it and then do not require the jdbc-mssql gem. This would make it possible to test with more recent drivers without an external dependency. What do you think?

dub357 commented 2 years ago

I'd like to revive this pull request. When this driver is used and the connection is JNDI, the application server can effectively wrap the connection (as is the case with Tomcat's connection pools - DBCP). In these cases, the connection is not a "com.microsoft.sqlserver.jdbc.SQLServerDriver" but rather a wrapper and the setSendTimeAsDatetime method is not available so errors are thrown:

undefined method 'setSendTimeAsDatetime' activerecord-jdbcsqlserver-adapter-52.0.0/lib/active_record/connection_adapters/sqlserver/jdbc_overrides.rb:103:in 'configure_connection'

rdubya commented 2 years ago

I haven't tried to set this up with a JNDI connection, if you want to put together a test case for it I can look at it. I haven't dug into this project in a long time, but I'm fairly certain that some date types will not function correctly without this being called.