Latest version of ARel conflicts with Sequel #133

Closed
joevandyk opened this Issue Aug 13, 2012 · 12 comments

Projects

None yet

5 participants

@joevandyk

I'm using Sequel and Rails in a project. Upgrading to Rails 3.2.8 from 3.2.6 caused the following backtrace:

> User.group('username').count
   (0.5ms)  SHOW max_identifier_length
TypeError: Cannot visit Sequel::SQL::AliasedExpression
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/gems/arel-3.0.2/lib/arel/visitors/visitor.rb:25:in `rescue in visit'
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/gems/arel-3.0.2/lib/arel/visitors/visitor.rb:19:in `visit'
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/gems/arel-3.0.2/lib/arel/visitors/to_sql.rb:134:in `block in visit_Arel_Nodes_SelectCore'
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/gems/arel-3.0.2/lib/arel/visitors/to_sql.rb:134:in `map'
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/gems/arel-3.0.2/lib/arel/visitors/to_sql.rb:134:in `visit_Arel_Nodes_SelectCore'
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/gems/arel-3.0.2/lib/arel/visitors/to_sql.rb:121:in `block in visit_Arel_Nodes_SelectStatement'
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/gems/arel-3.0.2/lib/arel/visitors/to_sql.rb:121:in `map'
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/gems/arel-3.0.2/lib/arel/visitors/to_sql.rb:121:in `visit_Arel_Nodes_SelectStatement'
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/gems/arel-3.0.2/lib/arel/visitors/visitor.rb:19:in `visit'
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/gems/arel-3.0.2/lib/arel/visitors/visitor.rb:5:in `accept'
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/gems/arel-3.0.2/lib/arel/visitors/to_sql.rb:19:in `accept'
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/bundler/gems/rails-c6854813c824/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:7:in `to_sql'
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/bundler/gems/rails-c6854813c824/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:18:in `select_all'
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/bundler/gems/rails-c6854813c824/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb:63:in `select_all'
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/bundler/gems/rails-c6854813c824/activerecord/lib/active_record/relation/calculations.rb:289:in `execute_grouped_calculation'
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/bundler/gems/rails-c6854813c824/activerecord/lib/active_record/relation/calculations.rb:206:in `perform_calculation'
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/bundler/gems/rails-c6854813c824/activerecord/lib/active_record/relation/calculations.rb:159:in `calculate'
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/bundler/gems/rails-c6854813c824/activerecord/lib/active_record/relation/calculations.rb:58:in `count'
    from (irb):1
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/bundler/gems/rails-c6854813c824/railties/lib/rails/commands/console.rb:47:in `start'
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/bundler/gems/rails-c6854813c824/railties/lib/rails/commands/console.rb:8:in `start'
    from /Users/joe/projects/tanga/bundler/ruby/1.9.1/bundler/gems/rails-c6854813c824/railties/lib/rails/commands.rb:41:in `<top (required)>'
    from script/rails:6:in `require'
    from script/rails:6

I believe this is the cause: jeremyevans/sequel#532

@jeremyevans

I'm not sure this is an Arel bug, since the buggy code is in ActiveRecord. The issue was introduced in rails/rails@a1c05dd. Looking at that commit, there's definitely a reason for the change, so you aren't going to want to just revert it. Hopefully the respond_to?(:as) check can be switched to something better. @ernie, do you have any ideas?

@ernie

Yeah, there's definitely a reason for the change. @jeremyevans, is it safe for me to assume that since Sequel users who are also using AR are being bitten by this bug, that Sequel monkeypatches String with an implementation of #as?

I'm really loathe to go back to checking for included modules or such, though if we really, really needed to, some refactoring at the ARel level would let us check for inclusion of Arel::AliasPredication. Did I mention I'm not really OK with doing that, though?

@ernie

Answering my own question: yeah -- https://github.com/jeremyevans/sequel/blob/master/lib/sequel/extensions/core_extensions.rb#L173

Looks like that's the culprit. :(

@jeremyevans

Correct. Sequel can be used without any core extensions by requiring sequel/no_core_ext (a feature I don't believe ActiveRecord offers), but that's not going to be the default till Sequel 4.

FWIW, Sequel has defined String#as and Symbol#as for over 4 years, and this is the first case I'm aware of of something breaking because of it.

@ernie

Is it common to use both Sequel and AR in the same project? I always sort of imagined Sequel was more of an alternative to AR vs something you'd only use piecemeal.

@ernie

One thing to note, as a workaround, is that it's possible to teach ARel how to visit the Sequel::SQL::AliasedExpression, since it looks like your implementation of #as otherwise conforms to the same API as ARel's.

@ernie

If discussion is to continue on this, it probably makes sense to move it to the Rails issue tracker, with a reference to this issue. It's not an issue with ARel, really.

@ernie ernie closed this Aug 13, 2012
@joevandyk

@ernie a quick search of rails issues didn't turn up any tickets related to this .. you know if there's any fix?

@ernie

@joevandyk I wouldn't say this would be optimal but the only fix that comes to mind, if you must have Sequel's monkeypatches on String, it would be possible to check the as method's owner.

@joevandyk

BTW, for people who are coming across this bug, instead of

require 'sequel'
use
require 'sequel/no_core_ext'

And make sure that you don't have require 'sequel' anywhere.

This will remove sequel's monkeypatches on a couple core objects -- and you probably don't need those anyways. This will be the default behavior for sequel 4.0.

@veloper

If you're using the license_finder gem and it's in your Gemfile make sure to set :require => false option or else Sequel will be introduced into your Rails project alongside ActiveRecord.

@subdigital

+1, thanks @veloper -- that was the cause for me as well.

@adelevie adelevie referenced this issue in hale/rails_nlp Jun 16, 2014
Open

Feature/bronto error demo #3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment