NameError: method 'bind_block' not defined in Arel::Visitors::PostgreSQL & Java::JavaLang::NullPointerException

brianbroderick commented 4 years ago

I'm getting the Null Pointer Exception if I use 60.2. and the Arel issue if I use 61.0.pre1.

Versions:

Rails 6.0.3.4
activerecord-jdbcpostgresql-adapter (60.2-java)
JRuby 9.2.13.0 (I also tried 9.2.7)

Or

Rails 6.0.3.4
activerecord-jdbcpostgresql-adapter (61.0.pre1-java)
JRuby 9.2.13.0

Note: I'm using asdf as my Ruby Version Manager.

rails new jruby_activerecord_test and config database.yml or clone this repo
cd to this folder and run ruby -v Make sure you're using JRuby. The .tool-versions file sets it to jruby-9.2.13.0 I also tried jruby-9.2.7.0 and get the same error.
Add gem 'activerecord-jdbcpostgresql-adapter', platforms: [:jruby] to Gemfile. Don't add gem 'activerecord-jdbc-adapter', :github => 'jruby/activerecord-jdbc-adapter', platforms: [:jruby]. I'll show that error in a minute.
gem install bundler
bundle install You should get something like Bundle complete! 18 Gemfile dependencies, 75 gems now installed.
rake db:create or bundle exec rake db:create

Here is a test repo to reproduce. It also has the full stack trace.

https://github.com/brianbroderick/jruby_activerecord_test

Stack trace on 60.2

Couldn't create 'ar_dev' database. Please check your configuration.
~/.asdf/installs/ruby/jruby-9.2.13.0/lib/ruby/gems/shared/gems/rake-13.0.1/lib/rake/task.rb:230: warning: singleton on non-persistent Java type Java::JavaLang::NullPointerException (http://wiki.jruby.org/Persistence)
rake aborted!
Java::JavaLang::NullPointerException:
arjdbc.postgresql.PostgreSQLRubyJdbcConnection.newConnection(PostgreSQLRubyJdbcConnection.java:244)

Error on 61.0.pre1

$ bundle exec rake db:create
rake aborted!
NameError: method 'bind_block' not defined in Arel::Visitors::PostgreSQL
~/projects/jruby_activerecord_test/config/environment.rb:5:in `<main>'
~/.asdf/installs/ruby/jruby-9.2.13.0/bin/bundle:23:in `<main>'
Tasks: TOP => db:create => db:load_config => environment
(See full trace by running task with --trace)

headius commented 4 years ago

With the US election many of us are dark for a while, but we will look into this soon! Perhaps you can look around and try to figure out what "bind_block" is supposed to be?

brianbroderick commented 4 years ago

Thanks @headius

In the master branch, here's the reference:

https://github.com/rails/rails/blob/master/activerecord/lib/arel/visitors/postgresql.rb#L91

BIND_BLOCK = proc { |i| "$#{i}" }
private_constant :BIND_BLOCK

def bind_block; BIND_BLOCK; end

In the latest branch, it isn't there:

https://github.com/rails/rails/blob/v6.0.3.4/activerecord/lib/arel/visitors/postgresql.rb

There's not much going on with bind_block, so not sure what the point is.

headius commented 4 years ago

Aha, perhaps we added this and have not gotten it into a release? cc @enebo @kares

brianbroderick commented 4 years ago

https://github.com/jruby/activerecord-jdbc-adapter/blob/master/lib/arel/visitors/postgresql_jdbc.rb

In this repo here, it has this:

class Arel::Visitors::PostgreSQL
  # AREL converts bind argument markers "?" to "$n" for PG, but JDBC wants "?".
  remove_method :bind_block
end

This might be the real problem. Maybe we should overload the method instead?

headius commented 4 years ago

Maybe we should overload the method instead?

Yeah that might make more sense. I'm not sure why we remove it explicitly.

headius commented 4 years ago

You could try modifying your local copy to just replace bind_block with a definition that returns its single argument, which I think is what we intended here?

brianbroderick commented 4 years ago

yeah, i'll give that a try

brianbroderick commented 4 years ago

The PR above fixes that error. Now I'm seeing another error:

rake db:create
rake aborted!
NoMethodError: undefined method `database' for #<ActiveRecord::DatabaseConfigurations::UrlConfig:0x16ef1160>
~/projects/activerecord-jdbc-adapter/lib/arjdbc/tasks/databases.rake:16:in `block in each_current_configuration'
~/projects/activerecord-jdbc-adapter/lib/arjdbc/tasks/databases.rake:13:in `block in each_current_configuration'
~/projects/activerecord-jdbc-adapter/lib/arjdbc/tasks/databases.rake:12:in `each_current_configuration'

Not sure what's up with that one yet.

dr-itz commented 4 years ago

Thanks for providing a reproducer app! I did these changes and now it works for me:

diff --git a/Gemfile b/Gemfile
index 6663ad2..c5dd795 100644
--- a/Gemfile
+++ b/Gemfile
@@ -7,8 +7,8 @@ git_source(:github) { |repo| "https://github.com/#{repo}.git" }
 gem 'rails', '~> 6.0.3', '>= 6.0.3.4'
 # Use sqlite3 as the database for Active Record
 gem 'pg', platforms: [:mri]
-gem 'activerecord-jdbc-adapter', :github => 'jruby/activerecord-jdbc-adapter', platforms: [:jruby]
-gem 'activerecord-jdbcpostgresql-adapter', platforms: [:jruby]
+gem 'activerecord-jdbc-adapter', '~> 60.2', platforms: [:jruby]
+gem 'activerecord-jdbcpostgresql-adapter', '~> 60.2', platforms: [:jruby]
 # Use Puma as the app server
 gem 'puma', '~> 4.1'
 # Use SCSS for stylesheets
diff --git a/Gemfile.lock b/Gemfile.lock
index 6a417b0..aaf0621 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -1,11 +1,3 @@
-GIT
-  remote: https://github.com/jruby/activerecord-jdbc-adapter.git
-  revision: 0a3e5f48eeb7a1b1883c233dc344d3ddf3f740a9
-  specs:
-    activerecord-jdbc-adapter (61.0.pre1-java)
-      activerecord (~> 6.0.0)
-    jdbc-postgres (42.2.14)
-
 GEM
   remote: https://rubygems.org/
   specs:
@@ -53,9 +45,11 @@ GEM
     activerecord (6.0.3.4)
       activemodel (= 6.0.3.4)
       activesupport (= 6.0.3.4)
-    activerecord-jdbcpostgresql-adapter (0.8.2)
-      activerecord-jdbc-adapter (>= 0.8.2)
-      jdbc-postgres (>= 8.2)
+    activerecord-jdbc-adapter (60.2-java)
+      activerecord (~> 6.0.0)
+    activerecord-jdbcpostgresql-adapter (60.2-java)
+      activerecord-jdbc-adapter (= 60.2)
+      jdbc-postgres (>= 9.4, < 43)
     activestorage (6.0.3.4)
       actionpack (= 6.0.3.4)
       activejob (= 6.0.3.4)
@@ -96,6 +90,7 @@ GEM
       concurrent-ruby (~> 1.0)
     jbuilder (2.10.1)
       activesupport (>= 5.0.0)
+    jdbc-postgres (42.2.14)
     listen (3.2.1)
       rb-fsevent (~> 0.10, >= 0.10.3)
       rb-inotify (~> 0.9, >= 0.9.10)
@@ -223,8 +218,8 @@ PLATFORMS
   ruby

 DEPENDENCIES
-  activerecord-jdbc-adapter!
-  activerecord-jdbcpostgresql-adapter
+  activerecord-jdbc-adapter (~> 60.2)
+  activerecord-jdbcpostgresql-adapter (~> 60.2)
   bootsnap (>= 1.4.2)
   byebug
   capybara (>= 2.15)
diff --git a/config/database.yml b/config/database.yml
index cc8f0f0..434a298 100644
--- a/config/database.yml
+++ b/config/database.yml
@@ -10,7 +10,8 @@ development:
   adapter: postgresql
   encoding: unicode
   pool: 5
-  username: postgres
+  username: <%= ENV["DB_USERNAME"] || "postgres"  %>
+  password: <%= ENV["DB_PASSWORD"] || nil %>
   host: localhost
   database: ar_dev

Notes:

the activerecord-jdbc-* versions must match the Rails version. i.e. 60.x is for Rails 6.0.x, 61.x is for Rails 6.1.x
by not specifying a version at all, the wrong versions are installed, see changes in Gemfile.lock
in my setup I have to specify a password always

enebo commented 4 years ago

So based on PR comments and this last comment it looks like the issue was trying to use our master branch for rails 6.0 which is our branch for Rails 6.1. If anyone reading this wants to you we have Rails 6.0 code on our 60-stable branch.

headius commented 4 years ago

@enebo Maybe we should be warning somewhere if you try to load e.g. 60 under Rails 6.1? What happens if someone tries to use the CRuby adapters in this way?

enebo commented 4 years ago

@headius That is a good idea. I guess the only problem I could see would be that maybe AR will stabilize where we do work across versions? Nonetheless that has never happened so if that ends up being true we can just add a little more logic to not issue a warning.

brianbroderick commented 4 years ago

Thanks for looking at this.

jruby / activerecord-jdbc-adapter