jruby / activerecord-jdbc-adapter

JRuby's ActiveRecord adapter using JDBC.
BSD 2-Clause "Simplified" License
461 stars 385 forks source link

ArgumentError: wrong number of arguments (given 4, expected 1..3) #1013

Closed alaz closed 4 years ago

alaz commented 5 years ago

Hello,

I am facing an issue similar to #955 starting with version 52.1 and it is still a problem with 52.2. The versions are:

I do not experience this error with version 52.0

The stacktrace is below:

vendor/bundle/jruby/2.5.0/gems/activerecord-jdbc-adapter-52.2-java/lib/arjdbc/abstract/core.rb:12:in `initialize': wrong number of arguments (given 4, expected 1..3) (ArgumentError)
    from vendor/bundle/jruby/2.5.0/gems/activerecord-jdbc-adapter-52.2-java/lib/arjdbc/jdbc/callbacks.rb:12:in `new'
    from vendor/bundle/jruby/2.5.0/gems/activerecord-jdbc-adapter-52.2-java/lib/arjdbc/jdbc/connection_methods.rb:8:in `jdbc_connection'
    from vendor/bundle/jruby/2.5.0/gems/activerecord-jdbc-adapter-52.2-java/lib/arjdbc/mysql/connection_methods.rb:94:in `mysql_connection'

Basically what I am doing in my code is calling mysql_connection config.

Regards, Alexander

Confusion commented 5 years ago

I solved a similar problem by requiring arjdbc/db2 instead of arjdbc and changing adapter: 'jdbc' in the config to adapter: 'as400'. Perhaps you need similar changes to require and use specifically what you need instead of the generic jdbc adapter and code.

redconfetti commented 4 years ago

I've dug into this a bit and I see what's happening.

ActiveRecord 6.0.2 provides ActiveRecord::ConnectionAdapters::AbstractAdapter for any other adapter to inherit from.

When I'm trying to make an H2 connection, ActiveRecordJdbcAdapter v60.0 uses ArJdbc::ConnectionMethods.jdbc_connection to instantiate and return the conection adapter. It's passing 4 arguments.

In my case, the H2 adapter is ActiveRecord::ConnectionAdapters::H2Adapter, which inherits from ActiveRecord::ConnectionAdapters::JdbcAdapter, which in turn inherits from the ActiveRecord v6.0.2 version of the abstract adapter mentioned above.

I'm not totally sure why it works, but it appears that the reason why the AS400 adapter configuration mentioned above works is because it is following the 4 argument contract. I did this and the Oracle database connection worked fine.

Even for Rails v4.2.10 it appears that the AbstractAdapter required 3 arguments to initialize, not 4.

I see the @enebo made this change in a49ac47fa, related to #891, with the note:

Apparently my recent change to use using 3-arg constructors was a mistake piled onto the mistake that we made all top-level constructors be 3-arg constructors. In AR there is an extra 4th argument connection_parameters in all 3 adapters. Not matching AR means all extensions like postgis would need custom jruby code for a 3 arg constructor. This fix is to just adopt the 4-arg constructor.

I see that in ActiveRecord v6.0.2 that MySQL2Adapter and the AbstractMySQLAdapter that it inherits from, it takes 4 arguments to initialize. The same applies to PostgreSQLAdapter. I understand that these are likely the most common adapters used with a Rails app, and that Oracle and H2 is less common.

But in both cases, they make calls to super with 3 arguments.

headius commented 4 years ago

I think the answer may simply be that the H2 adapter needs updating!

In order to catch up compatibility, we had to refocus on the core three databases most folks are using with ActiveRecord: MySQL, Postgresql, and Sqlite. The resulting rework of arjdbc has made it much easier to maintain, but probably broke other adapters.

@redconfetti Since you've poked around in this code a bit, maybe you'd like to try your hand at a pull request?

redconfetti commented 4 years ago

Yeah. That's the direction I was heading in here @headius . I'm still doing some analysis to figure out what approach I should take. I'm actually wanting to fix this for both Oracle and H2, because using as400 as an adapter seems very hackey to me.

Unfortunately due to time constraints, I think we're just going to drop using the H2 database in our test environment.

headius commented 4 years ago

@redconfetti Unfortunate, but I understand. We want to support the popular embedded databases but with limited resources most of that work will have to come from the community. We will happily assist, though, and you can stop by our Matrix channel any time to chat!

headius commented 4 years ago

FWIW the new adapters are greatly simplified compared to the old ones, so maybe it wouldn't take too much work. @enebo would be able to offer some good pointers.

enebo commented 4 years ago

In the case of our current supported adapters we ended up trying to leverage the built-in core adapters existing Ruby code as much as was useful. For this adapter there will just be more code. With that said the base Java code you will be using was modified for Rails 5+ adapters so it is likely doing the basic things in a way you will want them to.

headius commented 4 years ago

I incorrectly closed this as legacy adapter support.

headius commented 4 years ago

This discussion got a bit off the rails, so to speak. The original report was against AR-JDBC 52 connecting to MySQL, which is still a configuration we support. The additional comments about the H2 and DB2 databases are a different issue; we no longer support those databases in ARJDBC 50+ due to lack of resources.

The issue reported here in ARJDBC 52 should still be fixed.

alaz commented 4 years ago

The problem still persists in AR-JDBC 60.2 (jruby 9.2.12)

2020-07-11T11:54:18.418+03:00 [Ruby-0-Thread-8@puma 001: /Users/alaz/.gem/ruby/2.7.0/gems/puma-3.12.6-java/lib/puma/thread_pool.rb:89] INFO TraceType : Exception raised: ArgumentError : wrong number of arguments (given 4, expected 1..3)
2020-07-11T11:54:18.421+03:00 [Ruby-0-Thread-8@puma 001: /Users/alaz/.gem/ruby/2.7.0/gems/puma-3.12.6-java/lib/puma/thread_pool.rb:89] INFO TraceType : Backtrace generated:
                      initialize at /Users/alaz/.gem/ruby/2.7.0/gems/activerecord-jdbc-adapter-60.2-java/lib/arjdbc/abstract/core.rb:14
                             new at /Users/alaz/.gem/ruby/2.7.0/gems/activerecord-jdbc-adapter-60.2-java/lib/arjdbc/jdbc/callbacks.rb:14
                 jdbc_connection at /Users/alaz/.gem/ruby/2.7.0/gems/activerecord-jdbc-adapter-60.2-java/lib/arjdbc/jdbc/connection_methods.rb:10
                mysql_connection at /Users/alaz/.gem/ruby/2.7.0/gems/activerecord-jdbc-adapter-60.2-java/lib/arjdbc/mysql/connection_methods.rb:99
               sphinx_connection at /Users/alaz/projects/admin-app/lib/active_record/connection_adapters/sphinx_adapter.rb:35
alaz commented 4 years ago

It has been a while since I opened this ticket. I finally managed to find some time to upgrade this codebase to Rails 6 and ActiveRecord JDBC 60.2

To be honest, I think I chose wrong path initially and this is a non issue. I managed to resolve my problem.


A bit of context follows. I am dealing with Sphinx Search, it is a full text search engine with SQL interface. I created a separate database configuration in Rails for it with a custom adapter:

config/sphinx.yml:

default: &default
  adapter: sphinx
  pool: <%= ENV['SPHINX_POOL_SIZE'] || 2 %>
  host: ...
  port: ....

and I was struggling to have an ActiveRecord::ConnectionHandling::sphinx_connection which would create a connection similar to MySQL, but working for Sphinx.

The final working variant for AR-JDBC 60.2 is something like

lib/active_record/connection_adapters/sphinx_adapter.rb:

module ActiveRecord
  module ConnectionHandling # :nodoc:
    def sphinx_connection(config)
      config = config.deep_dup
      config = config.symbolize_keys

      # Fine tune `config`

      adapter = ConnectionAdapters::Mysql2Adapter.prepend ConnectionAdapters::SphinxAdapter
      mysql_connection config.merge(adapter_class: adapter)
    end
  end

  module ConnectionAdapters
    # Sphinx adapter mixin
    module SphinxAdapter
      ADAPTER_NAME = "Sphinx".freeze

      private

      def configure_connection
        # sets up the connection for Sphinx
      end
    end
  end
Vinay50 commented 1 year ago

Hi @headius , Is this still open issue or it is fixed for AR 5.2?

Vinay50 commented 1 year ago

Hi @alaz Did it work with activerecord-jdbc-adapter: 52.2 ?

Vinay50 commented 1 year ago

hi @redconfetti do we except fix for this issue?