Rails 4.2 under Jruby can't find PGconn class

[rafaelfranca]

From @ninkibah on May 20, 2015 16:39

I am trying to upgrade our Rails 3.2 app to 4.2.1. So far, I've managed to get to 4.1.6 and I'm almost there. However, I am getting an
unknown constant ActiveRecord::ConnectionAdapters::PostgreSQL::OID:Bytea::PGconn

I suspect it has something to do with the ARJDBC adapter.

As a workaround, I added the following code to a config/initializer:

  module ConnectionAdapters
    module PostgreSQL
      module OID # :nodoc:
        class Bytea < Type::Binary # :nodoc:
          def type_cast_from_database(value)
            return if value.nil?
            return value.to_s if value.is_a?(Type::Binary::Data)
            val = super
            return val unless val.starts_with?('\x')
            [val[2..-1]].pack('H*')
          end
        end
      end
    end

The last two lines seem to be functionally equivalent to PGconn.unescape_bytea(super).

Question 1: Is this code as good as using the C/Java function which would normally be called? I presume pack is efficiently implemented in both Ruby and JRuby.

Question 2: Is this code worth doing a pull request for, or is it better to have the dependency on PGconn?

Copied from original issue: rails/rails#20229

[rafaelfranca]

or is it better to have the dependency on PGconn

We already depend o PGconn, so we should just use it normally here. Maybe we should change PGconn.unescape_bytea(super) to ::PGconn.unescape_bytea(super)?

[rafaelfranca]

From @prathamesh-sonpatki on May 20, 2015 17:44

PG gem is not used with JRuby, This code needs to be overridden in ActiveRecord JDBC side.

[rafaelfranca]

I see. I think we see where we use PGconn inside Rails adapter directly and isolate that in internal adapter calls so the JRuby adapter only override these isolated methods. @ninkibah do you want to work on this?

[rafaelfranca]

hmm, but I think the type objects should not know about the connection. So maybe your patch is fine. @sgrif WDYT?

[rafaelfranca]

From @sgrif on May 20, 2015 17:55

This shouldn't be new in 4.2, PGConn was used in 4.1 as well. I think this is a bug for the JRuby adapter. The type object in question is PG specific, so it should be allowed to make those sorts of assumptions. JDBC can provide its own type.

[rafaelfranca]

👍 I'll try to move the issue to their adapter

[prathamesh-sonpatki]

@rafaelfranca JRuby 4.2 support is not complete yet. I have tried to make it compatible but I don't have much understanding of AR internal changes related to Typecasting code. Can somebody from Rails team help to so that we can make it compatible? 😄

[prathamesh-sonpatki]

This is the issue for tracking progress on 4.2 #599

[rafaelfranca]

I can help with guidance, but I don't have time to work on code unfortunately 😢

[sgrif]

Same, I'm happy to answer questions, but don't have time to work on the code.

[prathamesh-sonpatki]

Thanks for the help. I will try to ask questions :) //cc @kares

[kares]

the issue was already reported previously (today) by @ninkibah as #651 so I'll keep that open instead (has a back-trace for tracking this one to AR's code)

Question 1: Is this code as good as using the C/Java function which would normally be called? I presume pack is efficiently implemented in both Ruby and JRuby.

hopefully - it's a very good start, although there might be better ways of doing these

Question 2: Is this code worth doing a pull request for, or is it better to have the dependency on PGconn?

no PGconn dependencies ... while there are attempts for a JRuby PG gem were not using pg at all

while yes, AR-JDBC is far from fully 4.2 compatible, I recall at some point in the past some of the pieces such as AR::CA::PostgreSQL::ArrayParser or AR::CA::PostgreSQL::OID were refactored so they can be shared with AR-JDBC, which is what we started doing (loading parts of the PostgreSQL adapter from AR) instead of copying them over as it used to be (and is when running AR < 4.0) but it seems a bit unreliable to do so 😿 ... hopefully there won't be more so that we do not have to switch back to ✂️

isn't there another way of not having any PG gem specific code under OID at all e.g. delegating OID::Bytea's type case escaping internals to the adapter [somewhere "outside"] which would than do the PGConn call (assuming there aren't more of course this would ease the patching needed on our end) ?

  module ConnectionAdapters
    module PostgreSQL
      module OID # :nodoc:
        class Bytea < Type::Binary # :nodoc:
          def type_cast_from_database(value)
            return if value.nil?
            return value.to_s if value.is_a?(Type::Binary::Data)
            PostgreSQL.unescape_bytea(super)
          end
        end
      end
    end

[sgrif]

You should be able to just create your own Bytea class which doesn't use PGConn, relatively trivially.

class JDBCPostgresqlAdapter < PostgreSQLAdapter
  def initialize_type_map(m)
    super
    m.register_type 'bytea', JDBCByteaType.new
  end
end

[kares]

@sgrif we're not a user of the type APIs instead we're actually trying to provide the best compatibility ... what if the user sub-classes the OID::Bytea or assumes it's there in the type map it's in the API after all.

also please drop the notion of AR-JDBC just overriding the core AR adapters that is the road to hell as AR-JDBC currently is ... not saying it's not possible but we do have a different hierarchy and support some Java specifics (such as JNDI connections). core AR while evolving is still not designed "well-enough" (not meaning it will or needs to be) to handle JDBC as an equal citizen.

thus at this point it would be really great if we could share some of the modules/helpers at least, but that would assume driver internals are not leaking-out or the adapter class too far.

[ninkibah]

@rafaelfranca I'll be happy to do the work, as this is critical to our project. But I will need direction.

Please all note that the breaking code is in ActiveRecord 4.2.1, and not in the ARJDBC code base. I admit it is confusing, as the OID classes in AR seem to be copies of the code in ARJDBC.

Also, I ran my test in a debugger, and put a breakpoint on the call to PGconn.unescape_bytea. At that point, ::PGconn is unknown as well. From memory, PGconn is defined in the pg gem as PG::Connection, but of course, under jruby, I'm not loading the pg gem. So now I'm wondering how any of this is supposed to work.

[ninkibah]

1. I notice in lib/arjdbc/postgresql/oid_types.rb, when used in Rails 4.2, we require the ActiveRecord OID module, instead of using our own:

    if AR42_COMPAT
      require 'active_record/connection_adapters/postgresql/oid'
    else
      require 'arjdbc/postgresql/base/oid'
    end

But even if we are using AR 3, we would load our own oid.rb, and for binary columns, we should invoke PGconn.unescape_bytea value. So, my question is, where does PGconn get defined? It should not be in the pg gem, as we shouldn't be using that under jruby.

What this suggests is that any solution will require defining a PGconn class/module with an unescape_bytea method.

Does this make sense?

[donv]

@ninkibah #655 should fix this for you. Could you try master?

[ninkibah]

@donv Thanks very much. All my tests pass with master!!!

BTW, I removed my workaround in config/initializers before running the tests :-)

[donv]

That's great, @ninkibah ! Glad to hear that 😄