Skip to content
This repository has been archived by the owner on Mar 30, 2022. It is now read-only.

op doesn't work with first #122

Closed
octavpo opened this issue May 22, 2012 · 17 comments
Closed

op doesn't work with first #122

octavpo opened this issue May 22, 2012 · 17 comments

Comments

@octavpo
Copy link

octavpo commented May 22, 2012

Course.where{starting_school_year.op('=', '2011')}.first

gives a TypeError: Cannot visit Arel::Nodes::InfixOperation in version 1.0.1, although it works fine in 1.0.0.

Thanks,
Octav

@ernie
Copy link
Contributor

ernie commented May 22, 2012

Can you provide a stack trace, and a copy of your Gemfile.lock? Also, what happens when you do:

Course.where{starting_school_year == 2011}.first

@octavpo
Copy link
Author

octavpo commented May 22, 2012

Actually I did some more testing and it seems it's not 'first' that gives the error, but 'empty?', 'any?', and the like. The strange thing is that after I get an error on 'empty?', 'first' will also give the same error, until I quit the console. And it works the same in 1.0.0 and 0.9.5, I was mislead by the fact that 'first' worked fine after I switched to 1.0.0 and restarted the console.

Course.where{starting_school_year == 2011}.empty?

works fine. Here's the stack trace for 'Course.where{starting_school_year.op('=', 2011)}.empty?' in 1.0.1:

TypeError: Cannot visit Arel::Nodes::InfixOperation
from /usr/local/lib/ruby/gems/1.8/gems/arel-3.0.2/lib/arel/visitors/visitor.rb:25:in visit' from /usr/local/lib/ruby/gems/1.8/gems/arel-3.0.2/lib/arel/visitors/to_sql.rb:323:invisit_Arel_Nodes_And'
from /usr/local/lib/ruby/gems/1.8/gems/arel-3.0.2/lib/arel/visitors/to_sql.rb:323:in map' from /usr/local/lib/ruby/gems/1.8/gems/arel-3.0.2/lib/arel/visitors/to_sql.rb:323:invisit_Arel_Nodes_And'
from /usr/local/lib/ruby/gems/1.8/gems/arel-3.0.2/lib/arel/visitors/visitor.rb:19:in send' from /usr/local/lib/ruby/gems/1.8/gems/arel-3.0.2/lib/arel/visitors/visitor.rb:19:invisit'
from /usr/local/lib/ruby/gems/1.8/gems/arel-3.0.2/lib/arel/visitors/to_sql.rb:136:in visit_Arel_Nodes_SelectCore' from /usr/local/lib/ruby/gems/1.8/gems/arel-3.0.2/lib/arel/visitors/to_sql.rb:136:inmap'
from /usr/local/lib/ruby/gems/1.8/gems/arel-3.0.2/lib/arel/visitors/to_sql.rb:136:in visit_Arel_Nodes_SelectCore' from /usr/local/lib/ruby/gems/1.8/gems/arel-3.0.2/lib/arel/visitors/mysql.rb:41:invisit_Arel_Nodes_SelectCore'
from /usr/local/lib/ruby/gems/1.8/gems/arel-3.0.2/lib/arel/visitors/to_sql.rb:121:in visit_Arel_Nodes_SelectStatement' from /usr/local/lib/ruby/gems/1.8/gems/arel-3.0.2/lib/arel/visitors/to_sql.rb:121:inmap'
from /usr/local/lib/ruby/gems/1.8/gems/arel-3.0.2/lib/arel/visitors/to_sql.rb:121:in visit_Arel_Nodes_SelectStatement' from /usr/local/lib/ruby/gems/1.8/gems/arel-3.0.2/lib/arel/visitors/mysql.rb:36:invisit_Arel_Nodes_SelectStatement'
from /usr/local/lib/ruby/gems/1.8/gems/arel-3.0.2/lib/arel/visitors/visitor.rb:19:in send' from /usr/local/lib/ruby/gems/1.8/gems/arel-3.0.2/lib/arel/visitors/visitor.rb:19:invisit'
from /usr/local/lib/ruby/gems/1.8/gems/arel-3.0.2/lib/arel/visitors/visitor.rb:5:in accept' from /usr/local/lib/ruby/gems/1.8/gems/arel-3.0.2/lib/arel/visitors/to_sql.rb:19:inaccept'
from /usr/local/lib/ruby/gems/1.8/gems/arel-3.0.2/lib/arel/visitors/bind_visitor.rb:11:in accept' from /usr/local/lib/ruby/gems/1.8/gems/activerecord-3.2.3/lib/active_record/connection_adapters/abstract/database_statements.rb:7:into_sql'
from /usr/local/lib/ruby/gems/1.8/gems/activerecord-3.2.3/lib/active_record/connection_adapters/abstract/database_statements.rb:18:in select_all' from /usr/local/lib/ruby/gems/1.8/gems/activerecord-3.2.3/lib/active_record/connection_adapters/abstract/query_cache.rb:63:inselect_all'
from /usr/local/lib/ruby/gems/1.8/gems/activerecord-3.2.3/lib/active_record/connection_adapters/abstract/database_statements.rb:24:in select_one' from /usr/local/lib/ruby/gems/1.8/gems/activerecord-3.2.3/lib/active_record/connection_adapters/abstract/database_statements.rb:30:inselect_value'
from /usr/local/lib/ruby/gems/1.8/gems/activerecord-3.2.3/lib/active_record/relation/calculations.rb:243:in execute_simple_calculation' from /usr/local/lib/ruby/gems/1.8/gems/activerecord-3.2.3/lib/active_record/relation/calculations.rb:208:inperform_calculation'
from /usr/local/lib/ruby/gems/1.8/gems/activerecord-3.2.3/lib/active_record/relation/calculations.rb:159:in calculate' from /usr/local/lib/ruby/gems/1.8/gems/activerecord-3.2.3/lib/active_record/relation/calculations.rb:58:incount'
from /usr/local/lib/ruby/gems/1.8/gems/activerecord-3.2.3/lib/active_record/relation.rb:210:in `empty?'

And the Gemfile.lock:

GEM
remote: https://rubygems.org/
specs:
actionmailer (3.2.3)
actionpack (= 3.2.3)
mail (> 2.4.4)
actionpack (3.2.3)
activemodel (= 3.2.3)
activesupport (= 3.2.3)
builder (
> 3.0.0)
erubis (> 2.7.0)
journey (
> 1.0.1)
rack (> 1.4.0)
rack-cache (
> 1.2)
rack-test (> 0.6.1)
sprockets (
> 2.1.2)
active_attr (0.5.1)
activemodel (>= 3.0.2, < 4.1)
activesupport (>= 3.0.2, < 4.1)
activemodel (3.2.3)
activesupport (= 3.2.3)
builder (> 3.0.0)
activerecord (3.2.3)
activemodel (= 3.2.3)
activesupport (= 3.2.3)
arel (
> 3.0.2)
tzinfo (> 0.3.29)
activeresource (3.2.3)
activemodel (= 3.2.3)
activesupport (= 3.2.3)
activesupport (3.2.3)
i18n (
> 0.6)
multi_json (> 1.0)
arel (3.0.2)
awesome_nested_set (2.1.3)
activerecord (>= 3.0.0)
builder (3.0.0)
capistrano (2.12.0)
highline
net-scp (>= 1.0.0)
net-sftp (>= 2.0.0)
net-ssh (>= 2.0.14)
net-ssh-gateway (>= 1.1.0)
capistrano-ext (1.2.1)
capistrano (>= 1.0.0)
coffee-rails (3.2.2)
coffee-script (>= 2.2.0)
railties (
> 3.2.0)
coffee-script (2.2.0)
coffee-script-source
execjs
coffee-script-source (1.3.3)
columnize (0.3.6)
erubis (2.7.0)
exception_notification (2.6.1)
actionmailer (>= 3.0.4)
execjs (1.4.0)
multi_json (> 1.0)
haml (3.1.6)
haml-rails (0.3.4)
actionpack (
> 3.0)
activesupport (> 3.0)
haml (
> 3.0)
railties (> 3.0)
highline (1.6.12)
hike (1.2.1)
i18n (0.6.0)
journey (1.0.3)
jquery-auto-session-timeout (0.5.1)
jquery-rails (2.0.2)
railties (>= 3.2.0, < 5.0)
thor (
> 0.14)
jruby-pageant (1.0.2)
json (1.7.3)
libv8 (3.3.10.4)
linecache (0.46)
rbx-require-relative (> 0.0.4)
mail (2.4.4)
i18n (>= 0.4.0)
mime-types (> 1.16)
treetop (
> 1.4.8)
mime-types (1.18)
multi_json (1.3.5)
mysql2 (0.3.11)
net-scp (1.0.4)
net-ssh (>= 1.99.1)
net-sftp (2.0.5)
net-ssh (>= 2.0.9)
net-ssh (2.4.0)
jruby-pageant (>= 1.0.2)
net-ssh-gateway (1.1.0)
net-ssh (>= 1.99.1)
nokogiri (1.5.2)
polyamorous (0.5.0)
activerecord (> 3.0)
polyglot (0.3.3)
rack (1.4.1)
rack-cache (1.2)
rack (>= 0.4)
rack-ssl (1.3.2)
rack
rack-test (0.6.1)
rack (>= 1.0)
rails (3.2.3)
actionmailer (= 3.2.3)
actionpack (= 3.2.3)
activerecord (= 3.2.3)
activeresource (= 3.2.3)
activesupport (= 3.2.3)
bundler (
> 1.0)
railties (= 3.2.3)
railties (3.2.3)
actionpack (= 3.2.3)
activesupport (= 3.2.3)
rack-ssl (> 1.3.2)
rake (>= 0.8.7)
rdoc (
> 3.4)
thor (> 0.14.6)
rake (0.9.2.2)
rbx-require-relative (0.0.9)
rdoc (3.12)
json (
> 1.4)
ruby-debug (0.10.4)
columnize (>= 0.1)
ruby-debug-base (> 0.10.4.0)
ruby-debug-base (0.10.4)
linecache (>= 0.3)
ruby-recaptcha (1.0.5)
sass (3.1.18)
sass-rails (3.2.5)
railties (
> 3.2.0)
sass (>= 3.1.10)
tilt (> 1.3)
sprockets (2.1.3)
hike (
> 1.2)
rack (> 1.0)
tilt (
> 1.1, != 1.3.0)
squeel (1.0.1)
activerecord (> 3.0)
activesupport (
> 3.0)
polyamorous (> 0.5.0)
swfheader (0.22)
therubyracer (0.10.1)
libv8 (
> 3.3.10)
thor (0.14.6)
tilt (1.3.3)
treetop (1.4.10)
polyglot
polyglot (>= 0.3.1)
tzinfo (0.3.33)
uglifier (1.2.4)
execjs (>= 0.3.0)
multi_json (>= 1.0.2)
yaml_db (0.2.3)

PLATFORMS
ruby

DEPENDENCIES
active_attr
awesome_nested_set
capistrano-ext
coffee-rails (> 3.2.2)
exception_notification
haml-rails
jquery-auto-session-timeout
jquery-rails
json
mysql2
nokogiri
rails (= 3.2.3)
ruby-debug
ruby-recaptcha
sass-rails (
> 3.2.5)
squeel
swfheader
therubyracer
uglifier
yaml_db

Thanks,
Octav

@ernie
Copy link
Contributor

ernie commented May 22, 2012

So, the issue would be coming from the attempt at a count, when asking for any? etc. Trying to duplicate it using rake console with my current master version, and not having any luck:

 => ActiveRecord::VERSION 
1.9.3p194 :008 > ActiveRecord::VERSION::STRING
 => "3.2.3" 
1.9.3p194 :009 > Squeel::VERSION
 => "1.0.1" 
1.9.3p194 :010 > Person.where{name.op('=', 'Ernie')}.first
 => nil 
1.9.3p194 :011 > Person.where{name.op('=', 'Ernie')}.any?
 => false 
1.9.3p194 :012 > Person.where{name.op('=', 'Ernie')}.count
 => 0 
1.9.3p194 :013 > Person.where{name.op('=', 'Ernie')}.empty?
 => true 
1.9.3p194 :014 > 

I do notice there's an "and" in your SQL though. Is it possible you have something in a default scope that might be causing issues? What do you get from:

Course.where{starting_school_year == 2011}.to_sql

@octavpo
Copy link
Author

octavpo commented May 22, 2012

Course.where{starting_school_year == 2011}.to_sql

gives

"SELECT courses.* FROM courses WHERE courses.starting_school_year = 2011"

I don't have a default scope on this model. So I don't know where the 'and' could come from. Any other tests I can run?

@octavpo
Copy link
Author

octavpo commented May 22, 2012

I tried it with some other models and other attributes, and it's the same for all of them. It starts to look like a conflict with one of the other gems. I'll try to remove some of them and see if it fixes it.

@ernie
Copy link
Contributor

ernie commented May 22, 2012

It almost has to be. Taking a look at the ARel source:

https://github.com/rails/arel/blob/master/lib/arel/visitors/to_sql.rb#L460-467

You can see that it most definitely does know how to visit.

@octavpo
Copy link
Author

octavpo commented May 23, 2012

I removed all the gems that were not requested directly by rails and mysql2, and I was still getting the same error. Then I removed everything and reinstalled rails and all the gems, and still the same.

So then I put a few trace messages in the 'visit' function that's raising the error. I ran the line:

User.where{username.op('=', 'test')}.count

Here's what I get right before the error:

Visitor#visit:
- object = #<Arel::Nodes::InfixOperation:0x1c0b0c4 @right="'test'", @operator="=", @left="`users`.`username`">
- class = Arel::Nodes::InfixOperation
- dispatch[object.class] = "visit_Arel_Nodes_Node"

So the problem is that the dispatch method for Arel::Nodes::InfixOperation in the hash is not visit_Arel_Nodes_InfixOperation, but visit_Arel_Nodes_Node. Any idea who's setting it? Here's the whole hash:

Visitor#dispatch:
- DISPATCH = {Enumerable=>"visit_Enumerable", Arel::Nodes::Binary=>"visit_Arel_Nodes_Binary", Arel::AliasPredication=>"visit_Arel_AliasPredication", Arel::Nodes::InfixOperation=>"visit_Arel_Nodes_Node", Arel::Nodes::JoinSource=>"visit_Arel_Nodes_JoinSource", String=>"visit_String", Object=>"visit_Object", Arel::Nodes::Count=>"visit_Arel_Nodes_Count", Arel::Nodes::And=>"visit_Arel_Nodes_And", Array=>"visit_Array", Arel::FactoryMethods=>"visit_Arel_FactoryMethods", Arel::Attributes::Attribute=>"visit_Arel_Attributes_Attribute", Symbol=>"visit_Symbol", NilClass=>"visit_NilClass", Arel::Expressions=>"visit_Arel_Expressions", Arel::Math=>"visit_Arel_Math", JSON::Ext::Generator::GeneratorMethods::Object=>"visit_JSON_Ext_Generator_GeneratorMethods_Object", Arel::OrderPredications=>"visit_Arel_OrderPredications", Arel::Nodes::SelectStatement=>"visit_Arel_Nodes_SelectStatement", ActiveSupport::Dependencies::Loadable=>"visit_ActiveSupport_Dependencies_Loadable", Rake::DeprecatedObjectDSL=>"visit_Rake_DeprecatedObjectDSL", Arel::Nodes::SqlLiteral=>"visit_Arel_Nodes_SqlLiteral", Arel::Table=>"visit_Arel_Table", Base64::Deprecated=>"visit_Base64_Deprecated", PP::ObjectMixin=>"visit_PP_ObjectMixin", Arel::Nodes::Node=>"visit_Arel_Nodes_Node", Arel::Predications=>"visit_Arel_Predications", Arel::Nodes::SelectCore=>"visit_Arel_Nodes_SelectCore"}

@octavpo
Copy link
Author

octavpo commented May 23, 2012

Never mind that, it seems it's set in the rescue part of 'visit', in an earlier call, when the original send failed. So the question is why the send fails.

Visitor#visit:
- object = #<Arel::Nodes::InfixOperation:0x1c277c4 @right="'test'", @operator="=", @left="`users`.`username`">
- class = Arel::Nodes::InfixOperation
- dispatch[object.class] = "visit_Arel_Nodes_InfixOperation"
***in rescue: - superklass = Arel::Nodes::Node

@ernie
Copy link
Contributor

ernie commented May 23, 2012

@octavpo, I'm sorry, but I really don't know what to say. I have tried to duplicate the issue, and failed. Unless we can work up a failing spec or something I can go on, I'm not sure the issue is even mine to fix. :(

@octavpo
Copy link
Author

octavpo commented May 23, 2012

One thing that might explain why you're not getting the error is that I'm using mysql, maybe you're using something else?

I did a little more tracing of Arel::Visitor::visit, looking at what object it's called on, and here's what happens: there are two attempts to build the relation. On the first attempt the visit to node Arel::Nodes::InfixOperation is called on a Arel::Visitors::DepthFirst object.

***Visitor::visit:
- self.class = Arel::Visitors::DepthFirst
- object = #<Arel::Nodes::InfixOperation:0x1c16bb8 @right="'test'", @operator="=", @left="`users`.`username`">
- class = Arel::Nodes::InfixOperation
- dispatch[object.class] = "visit_Arel_Nodes_InfixOperation"
***Visitor::visit in rescue: - superklass = Arel::Nodes::Node
***Visitor::visit:
- self.class = Arel::Visitors::DepthFirst
- object = #<Arel::Nodes::InfixOperation:0x1c16bb8 @right="'test'", @operator="=", @left="`users`.`username`">
- class = Arel::Nodes::InfixOperation
- dispatch[object.class] = "visit_Arel_Nodes_Node"

This one doesn't have a 'visit_Arel_Nodes_InfixOperation' method, so the rescue part in 'visit' looks for an ancestor and finds 'visit_Arel_Nodes_Node'. Which succeeds but does nothing. So the first attempt fails and then on the second attempt the visit to node Arel::Nodes::InfixOperation is called on a ActiveRecord::ConnectionAdapters::AbstractMysqlAdapter::BindSubstitution object.

***Visitor::visit:
- object = #<Arel::Nodes::InfixOperation:0x1bfcefc @right="'test'", @operator="=", @left="`users`.`username`">
- class = Arel::Nodes::InfixOperation
- dispatch[object.class] = "visit_Arel_Nodes_Node"

Which does have a 'visit_Arel_Nodes_InfixOperation' since it inherits from Arel::Visitors::MySQL, but the first visit replaced it with 'visit_Arel_Nodes_Node' in the dispatch hash, so it fails again.

So I tried to remove the last two lines

        dispatch[object.class] = dispatch[superklass]
        retry

from Arel::Visitor::visit, and it works, now the second attempt succeeds. I don't know why Arel::Visitors::DepthFirst comes into play for me and not for you, maybe it's mysql or maybe I have something else that gets in between. For now I can avoid it by using 'exists?' instead of 'empty?', or removing those lines if I really need a count function.

Thanks,
Octav

@ernie
Copy link
Contributor

ernie commented May 23, 2012

Try adding this to ARel's depth_first.rb:

      def visit_Arel_Nodes_InfixOperation o
        visit o.left
        visit o.operator
        visit o.right
      end

@octavpo
Copy link
Author

octavpo commented May 23, 2012

Yes that works. So it's a bug in Arel? Why weren't you getting it?

Thanks,
Octav

@ernie
Copy link
Contributor

ernie commented May 23, 2012

The test suite runs against sqlite3. I guess the depth-first visitor isn't needed in that case. I'll submit a pull request to ARel with the fix. Doing a mention to @tenderlove so he'll know it's coming in advance. ❤️ tenderlove

@ernie
Copy link
Contributor

ernie commented May 23, 2012

Hm. Never mind. I see the following in the code:

alias :visit_Arel_Nodes_InfixOperation     :binary

Sorry for the noise, @tenderlove.

@ernie
Copy link
Contributor

ernie commented May 23, 2012

@octavpo what that means is that the visit_Arel_Nodes_InfixOperation was already defined in that class. That makes me wonder why redefining it fixed your problem.

@octavpo
Copy link
Author

octavpo commented May 23, 2012

I don't have that alias line in the released arel 3.0.2 I have installed. Maybe they added it later? I tried it instead of your fix and it works too. Actually even an empty function seems to work, because it just prevents Visitor::visit to replace the dispatch function. So I'm not sure what's with DepthFirst. If it's supposed to build something it probably still fails, since it's still the second visit that succeeds.

@ernie
Copy link
Contributor

ernie commented May 23, 2012

DepthFirst visitors are there for the ability to iterate through an AST in the same way you might an array. And sure enough, looks like it was added in the past 2 months or so.

I could maybe do another conditional monkeypatch in compat.rb, I suppose.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants