activerecord-hackery / squeel

Active Record, improved. Live again :)
http://erniemiller.org/2013/11/17/anyone-interested-in-activerecord-hackery/
MIT License
2.4k stars 214 forks source link

Default scope where conditions with or in rails 4 and squeel #416

Open kbaum opened 8 years ago

kbaum commented 8 years ago

Hi. We are running into a problem with default scopes with or conditions when squeel is required. This problem can be reproduced easily from this project:

https://github.com/viewthespace/squeel_default_scope_or_condition_with_joins

I go into detail within the readme but to sum up, a default scope with an or condition and additional where criteria causes queries with joins from a parent to be malformed. The problem occurs without us using any squeel syntax. The project contains a test that demonstrates the problem only occurs with squeel required.

thx! -karl

kbaum commented 8 years ago

Just an update on this one. The problem seems to be in the collapse_where method. If i simply comment this monkey patch out, then the test passes.

kbaum commented 8 years ago

Update again. The problem is now reproduced within a spec on a fork of squeel.

https://github.com/viewthespace/squeel/commit/5571d4a96eb9d52f9427172acff6685b1692873f

As of now i am bit clueless of what the collapse_where method does and how to address the problem. Any advice would be greatly appreciated.

Thx!

azhi commented 8 years ago

Hey, it seems like the same issue as #361 and #411. PR #362 fixes it.

kbaum commented 8 years ago

@azhi - that looks to be the same issue.

I noticed that the issue is resolved if i simply comment out the collapse_where method. Is this monkey patch even needed in rails 4?

When I comment out the collapse_where method, the following tests fail:

rspec ./spec/squeel/adapters/active_record/relation_extensions_spec.rb:145 # Squeel::Adapters::ActiveRecord::RelationExtensions#build_arel combines multiple conditions of the same type against the same column with AND
rspec ./spec/squeel/adapters/active_record/relation_extensions_spec.rb:807 # Squeel::Adapters::ActiveRecord::RelationExtensions#joins joins an ActiveRecord::Relation subquery
rspec ./spec/squeel/adapters/active_record/relation_extensions_spec.rb:835 # Squeel::Adapters::ActiveRecord::RelationExtensions#joins default scopes with multiple wheres

But at a glance it does not look like the queries produced are invalid even though they do not exactly match expectations.

) Squeel::Adapters::ActiveRecord::RelationExtensions#build_arel combines multiple conditions of the same type against the same column with AND
     Failure/Error: arel.to_sql.should match /#{Q}people#{Q}.#{Q}name#{Q} [I]*LIKE '%bob%' AND #{Q}people#{Q}.#{Q}name#{Q} [I]*LIKE '%joe%'/
       expected "SELECT \"people\".* FROM \"people\" WHERE (\"people\".\"name\" LIKE '%bob%') AND (\"people\".\"name\" LIKE '%joe%')" to match /"people"."name" [I]*LIKE '%bob%' AND "people"."name" [I]*LIKE '%joe%'/
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-expectations-2.6.0/lib/rspec/expectations/fail_with.rb:29:in `fail_with'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-expectations-2.6.0/lib/rspec/expectations/handler.rb:21:in `handle_matcher'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-expectations-2.6.0/lib/rspec/expectations/extensions/kernel.rb:27:in `should'
     # ./spec/squeel/adapters/active_record/relation_extensions_spec.rb:149:in `block (3 levels) in <module:ActiveRecord>'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example.rb:48:in `instance_eval'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example.rb:48:in `block in run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example.rb:107:in `with_around_hooks'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example.rb:45:in `run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:294:in `block in run_examples'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:290:in `map'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:290:in `run_examples'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:262:in `run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:263:in `block in run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:263:in `map'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:263:in `run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/command_line.rb:24:in `block (2 levels) in run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/command_line.rb:24:in `map'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/command_line.rb:24:in `block in run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/reporter.rb:12:in `report'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/command_line.rb:21:in `run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/runner.rb:80:in `run_in_process'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/runner.rb:69:in `run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/runner.rb:11:in `block in autorun'

  2) Squeel::Adapters::ActiveRecord::RelationExtensions#joins joins an ActiveRecord::Relation subquery
     Failure/Error: relation.debug_sql.should match /SELECT #{Q}seat_order_items#{Q}.#{Q}amount#{Q}, seats.\* FROM #{Q}seats#{Q} LEFT OUTER JOIN #{Q}payments#{Q} ON #{Q}payments#{Q}.#{Q}id#{Q} = #{Q}seats#{Q}.#{Q}payment_id#{Q} LEFT OUTER JOIN \(SELECT #{Q}order_items#{Q}.#{Q}orderable_id#{Q}, sum\(#{Q}order_items#{Q}.#{Q}quantity#{Q} \* #{Q}order_items#{Q}.#{Q}unit_price#{Q}\) AS amount FROM #{Q}order_items#{Q}\s+GROUP BY #{Q}order_items#{Q}.#{Q}orderable_id#{Q}\) seat_order_items ON #{Q}seats#{Q}.#{Q}id#{Q} = #{Q}seat_order_items#{Q}.#{Q}orderable_id#{Q} WHERE #{Q}seat_order_items#{Q}.#{Q}amount#{Q} > 0/
       expected "SELECT \"seat_order_items\".\"amount\", seats.* FROM \"seats\" LEFT OUTER JOIN \"payments\" ON \"payments\".\"id\" = \"seats\".\"payment_id\" LEFT OUTER JOIN (SELECT \"order_items\".\"orderable_id\", sum(\"order_items\".\"quantity\" * \"order_items\".\"unit_price\") AS amount FROM \"order_items\" GROUP BY \"order_items\".\"orderable_id\") seat_order_items ON \"seats\".\"id\" = \"seat_order_items\".\"orderable_id\" WHERE (\"seat_order_items\".\"amount\" > 0)" to match /SELECT "seat_order_items"."amount", seats.\* FROM "seats" LEFT OUTER JOIN "payments" ON "payments"."id" = "seats"."payment_id" LEFT OUTER JOIN \(SELECT "order_items"."orderable_id", sum\("order_items"."quantity" \* "order_items"."unit_price"\) AS amount FROM "order_items"\s+GROUP BY "order_items"."orderable_id"\) seat_order_items ON "seats"."id" = "seat_order_items"."orderable_id" WHERE "seat_order_items"."amount" > 0/
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-expectations-2.6.0/lib/rspec/expectations/fail_with.rb:29:in `fail_with'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-expectations-2.6.0/lib/rspec/expectations/handler.rb:21:in `handle_matcher'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-expectations-2.6.0/lib/rspec/expectations/extensions/kernel.rb:27:in `should'
     # ./spec/squeel/adapters/active_record/relation_extensions_spec.rb:818:in `block (3 levels) in <module:ActiveRecord>'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example.rb:48:in `instance_eval'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example.rb:48:in `block in run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example.rb:107:in `with_around_hooks'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example.rb:45:in `run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:294:in `block in run_examples'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:290:in `map'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:290:in `run_examples'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:262:in `run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:263:in `block in run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:263:in `map'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:263:in `run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/command_line.rb:24:in `block (2 levels) in run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/command_line.rb:24:in `map'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/command_line.rb:24:in `block in run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/reporter.rb:12:in `report'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/command_line.rb:21:in `run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/runner.rb:80:in `run_in_process'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/runner.rb:69:in `run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/runner.rb:11:in `block in autorun'

  3) Squeel::Adapters::ActiveRecord::RelationExtensions#joins default scopes with multiple wheres
     Failure/Error: relation.to_sql.should eq "

       expected "SELECT \"depts\".* FROM \"depts\" INNER JOIN \"people\" ON \"people\".\"dept_id\" = \"depts\".\"id\" AND \"people\".\"name\" = 'Bill' AND \"people\".\"salary\" < 20000"
            got "SELECT \"depts\".* FROM \"depts\" INNER JOIN \"people\" ON \"people\".\"dept_id\" = \"depts\".\"id\" AND \"people\".\"name\" = 'Bill' AND (\"people\".\"salary\" < 20000)"

       (compared using ==)
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-expectations-2.6.0/lib/rspec/expectations/fail_with.rb:29:in `fail_with'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-expectations-2.6.0/lib/rspec/expectations/handler.rb:19:in `handle_matcher'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-expectations-2.6.0/lib/rspec/expectations/extensions/kernel.rb:27:in `should'
     # ./spec/squeel/adapters/active_record/relation_extensions_spec.rb:839:in `block (3 levels) in <module:ActiveRecord>'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example.rb:48:in `instance_eval'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example.rb:48:in `block in run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example.rb:107:in `with_around_hooks'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example.rb:45:in `run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:294:in `block in run_examples'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:290:in `map'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:290:in `run_examples'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:262:in `run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:263:in `block in run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:263:in `map'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/example_group.rb:263:in `run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/command_line.rb:24:in `block (2 levels) in run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/command_line.rb:24:in `map'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/command_line.rb:24:in `block in run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/reporter.rb:12:in `report'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/command_line.rb:21:in `run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/runner.rb:80:in `run_in_process'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/runner.rb:69:in `run'
     # /Users/kb/.rvm/gems/ruby-2.1.5/gems/rspec-core-2.6.4/lib/rspec/core/runner.rb:11:in `block in autorun'

Finished in 1.6 seconds
959 examples, 3 failures, 7 pending

I self-admittedly do not know what the collapse_where monkey patch does so i may be way off base.

thx!