Support JRuby w/JDBC Connection Mode

[iaddict]

I read in the documentation that this gem has "no strict gem dependencies outside of ActiveRecord".
But the gemspec lists tiny_tds as a dependency.

Would it be possible to remove this dependency?

[iaddict]

Some background: I started to modify activerecord-sqlserver-adapter to also support sequel (aka the sequel gem) as a connection mode. With this it's possible to use it with jruby and a jdbc connection. A basic jruby on rails application with some crud actions already works just fine.

I found that this would be an alternative to activerecord-jdbcmssql-adapter which as of now has no support for ActiveRecord 5 and also has several sql issues which are unfixed for quite a while.

Would you be interested in a pull request?

[metaskills]

Some history, the adapter for a long time supported 2 and something 3 connection modes. The legacy ODBC mode has never been tested or supported for at least 3 years. So it was removed in Rails v5. Before then, I heavily advocated that this adapter could be a rally point for talking to SQL Server from any raw connection as long as we took the high road of using TSQL for everything (can go into that more if needed). During that time I even added IronRuby support. I even created tickets to reach out to jruby/jdbc that were unfruitful.

A few weeks ago I created (and closed) a rails-core issue as part of a series of making getting started with SQL Server vNext/Linux super easy using Rails generators database option. Because we only have one connection mode (and a small constraint in Rails) it was an easy decision to make TinyTDS a dependency. The goal being to allow SUPER EASY setup for those getting started with Ruby/Rails and SQL Server. In a few weeks, we will have Ruby/Rails added to the Microsoft SQL Server Getting Started page and the directions now avoid the "now add TinyTDS to your gem file steps" which is kind of cool.

Where do we go from here? I still believe this adapter would be stronger with more backends and we had a unified approach to talking to SQL Server via ActiveRecord. Part of me thinks that it would take seeing that come to fruition first would be needed to think about removing the dep now on TinyTDS. That trip would not be easy because there may be differences of how to do things for each connection mode that might make this code base quite complex, CI setup, etc. Can you read over this and the links and tell me what you think?

[iaddict]

Many thanks for your detailed explanation and thoughts!

Maybe we can keep the dependency on the tiny_tds gem and supply a platform dependent tiny_tds gem for java/jruby. Actually that's what I did. If you bundle with jruby this platform gem is automatically chosen, eg. tiny_tds-0.1.0-java.gem.

My implementation of this gem has almost no code. It just checks if sequel can be required and gives some hints if this is not possible.

What do you think about this approach?

[metaskills]

It sounds hot! Mind sharing some code/prs now?

[iaddict]

Ok. Here we go:

https://github.com/iaddict/tiny_tds-java
https://github.com/iaddict/activerecord-sqlserver-adapter/tree/sequel-mode

I ran the tests on a SQL-Server 2016 which currently finishes with at least some success:

221 runs, 847 assertions, 7 failures, 20 errors, 5 skips

Some of the failing tests are related to permission denied errors (I'm not the sysadmin of this server).

To make bundle install succeed, you first need to build and install the tiny_tds-java gem:

jruby -S gem build tiny_tds-java.gemspec
jruby -S gem install tiny_tds-0.1.0-java.gem

To run the tests, I used the following environment:

export JRUBY_OPTS="--dev -X+O"
export ONLY_SQLSERVER=1
jruby -S bundle exec rake test:sequel

[metaskills]

Some of the failing tests are related to permission denied errors (I'm not the sysadmin of this server).

Why not use the Docker image for SQL Server that our CI uses? I do the same.
https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/blob/master/test/bin/setup.sh

https://github.com/iaddict/activerecord-sqlserver-adapter/tree/sequel-mode

Couldnt we use Jdbc vis the Sequel so the real raw connection is right to the metal? What is the thinking behind use the Sequel's wrapper?

[iaddict]

Sorry for the late answer.
Anyway – there is some progress with the sequel mode. The tests already look a lot more promising:

222 runs, 1126 assertions, 1 failures, 0 errors, 9 skips

The sequel gem already does a lot of the stuff that would have to be implemented a second time, like the type casts, connection pooling, reconnection handling, ...
I think - at least for now - it is much easier to stand on the shoulders of giants ;)

Maybe you can take a brief look at the commits and tell me if you think the route taken is ok from your point of view?

By the way - many thanks for the great work on this piece of software! I find it much more accessible then the AR-jdbc adapter.

[metaskills]

The sequel gem already does a lot of the stuff that would have to be implemented a second time

To be fair, this is work that ActiveRecord would do. Can we explore a solution without it and let ActiveRecord be the giant?

By the way - many thanks for the great work on this piece of software! I find it much more accessible then the AR-jdbc adapter.

Thanks!!! Means a lot to me and the others that contribute/use this.

[iaddict]

The sequel gem already does a lot of the stuff that would have to be implemented a second time

To be fair, this is work that ActiveRecord would do. Can we explore a solution without it and let ActiveRecord be the giant?

Well, I already had a look at the code in sequel and ar-jdbc that is required for the jdbc connection handling and the type conversion stuff. I found it not to be worth to write this once again and not use the battle tested sequel gem. The inspiration to use sequel came from the rom gem, which does the same.

Maybe we can at least see the sequel mode as an intermediate step. I can rename it to jdbc mode, but still delegate the heavy lifting to sequel for now. At a later time I could rewrite it to directly use jdbc. For a user the switch would then be transparent.

What do you think?

I would be very happy if you would consider a merge of this approach, as it brings a stable alternative for sqlserver/jruby users.

The AR test suite now also runs with not that many failures/errors:

4876 runs, 14121 assertions, 25 failures, 21 errors, 12 skips

[iaddict]

A small update on the progress of the imminent sequel mode ;)
4873 runs, 14159 assertions, 3 failures, 4 errors, 12 skips

[metaskills]

The inspiration to use sequel came from the rom gem, which does the same.

I think that makes sense for ROM, but not the adapter. The parts we need logically should belong to the adapter. Can we make that happen? I feel there are good interface points for this work and we should let ActiveRecord's type system work for us.

[iaddict]

I'm quite happy with the sequel mode so far and fear the burden of maintaining a completely self implemented tiny_tds-java gem a bit. Sequel already does a lot of stuff which essentially would have to be reimplemented. See: https://github.com/jeremyevans/sequel/blob/master/lib/sequel/adapters/jdbc.rb

Anyway – how would you envision this tiny_tds-java gem should look like? Wouldn't it just be an api compatible implementation of the tiny_tds gem? If this would be the case, can you supply a spec or abstract implementation of the minimal set of required methods to make implementation a bit easier?

[metaskills]

Sequel already does a lot of stuff which essentially would have to be reimplemented.

By contrast the TinyTDS adapter in Sequel is half the size of that jdbc file. The other half seems type related which the adapter should do for us. So I am still not seeing a lot that requiring a competing ORM buys us. I would very much prefer we implement a long jdbc_connect method that is our own and work past any connection/type issues as the come up.

how would you envision this tiny_tds-java gem

If the above were done, I would drop TinyTDS as a runtime dep and go back to the days of requiring a connection mode gem be specified by the end users.

FYI, I am gonna start some hard work in master for 5.1. Ideally, I think this PR should target 5-0-stable. What do you think?

[iaddict]

By contrast the TinyTDS adapter in Sequel is half the size of that jdbc file. The other half seems type related which the adapter should do for us.

That means activerecord-sqlserver-adapter should do the type conversion from Java types (eg. results on select) to ruby types?
I thought it will already return proper ruby types, thus the type conversion (at least from java to ruby) needs to be implemented.

[metaskills]

That means activerecord-sqlserver-adapter should do the type conversion from Java types (eg. results on select) to ruby types? I thought it will already return proper ruby types, thus the type conversion (at least from java to ruby) needs to be implemented.

I assumed that everything that came back from JDBC was or could be a string. In theory, if that was the case, then ActiveRecord would do the type conversions for us and most of sequel/adapters/jdbc would be moot. This is how MySQL, PG, etc do things. TinyTDS is a rare breed that does all the casting to Ruby primitives for us. But again, in theory, this is not needed with ActiveRecord's current state.