Fix tests to work properly on Ruby 1.9, honor multiple calls to #order in memory engine, and make having clauses behave like where clauses in SQL engine (join with AND, allow multiple calls to having to add predicates)

10 files changed, 93 insertions, 28 deletions

diff --git a/lib/arel/algebra/relations/operations/having.rb b/lib/arel/algebra/relations/operations/having.rb
index cd16535609..0cd86c284e 100644
--- a/lib/arel/algebra/relations/operations/having.rb
+++ b/lib/arel/algebra/relations/operations/having.rb
@@ -1,13 +1,17 @@
 module Arel
   class Having < Compound
-    attributes :relation, :havings
-    deriving :==
+    attributes :relation, :predicates
+    deriving   :==
+    requires   :restricting
 
-    def initialize(relation, *havings, &block)
-      @relation = relation
-      @havings = (havings + arguments_from_block(relation, &block)) \
-        .collect { |g| g.bind(relation) }
+    def initialize(relation, *predicates, &block)
+      predicates = [yield(relation)] + predicates if block_given?
+      @predicates = predicates.map { |p| p.bind(relation) }
+      @relation   = relation
+    end
+
+    def havings
+      @havings ||= relation.havings + predicates
     end
   end
 end
-
diff --git a/lib/arel/engines/memory/relations/operations.rb b/lib/arel/engines/memory/relations/operations.rb
index 55b4dcb677..75777a0c7f 100644
--- a/lib/arel/engines/memory/relations/operations.rb
+++ b/lib/arel/engines/memory/relations/operations.rb
@@ -8,7 +8,7 @@ module Arel
   class Order < Compound
     def eval
       unoperated_rows.sort do |row1, row2|
-        ordering = orderings.detect { |o| o.eval(row1, row2) != 0 } || orderings.last
+        ordering = orders.detect { |o| o.eval(row1, row2) != 0 } || orders.last
         ordering.eval(row1, row2)
       end
     end
diff --git a/lib/arel/engines/sql/relations/compiler.rb b/lib/arel/engines/sql/relations/compiler.rb
index cc0deb7c88..374967b7be 100644
--- a/lib/arel/engines/sql/relations/compiler.rb
+++ b/lib/arel/engines/sql/relations/compiler.rb
@@ -12,9 +12,9 @@ module Arel
           "SELECT     #{select_clauses.join(', ')}",
           "FROM       #{from_clauses}",
           (joins(self)                                   unless joins(self).blank? ),
-          ("WHERE     #{where_clauses.join(" AND ")}"    unless wheres.blank?      ),
+          ("WHERE     #{where_clauses.join(' AND ')}"    unless wheres.blank?      ),
           ("GROUP BY  #{group_clauses.join(', ')}"       unless groupings.blank?   ),
-          ("HAVING    #{having_clauses.join(', ')}"      unless havings.blank?     ),
+          ("HAVING    #{having_clauses.join(' AND ')}"      unless havings.blank?     ),
           ("ORDER BY  #{order_clauses.join(', ')}"       unless orders.blank?      )
           engine.add_limit_offset!(query,{ :limit => taken, :offset => skipped }) if taken || skipped
           query << " #{locked}" unless locked.blank?
diff --git a/spec/attributes/time_spec.rb b/spec/attributes/time_spec.rb
index fcbe4f58e5..044b98d6ec 100644
--- a/spec/attributes/time_spec.rb
+++ b/spec/attributes/time_spec.rb
@@ -16,7 +16,9 @@ module Arel
     end
 
     describe "#type_cast" do
-      it "works"
+      it "works" do
+        pending
+      end
     end
   end
 end
\ No newline at end of file
diff --git a/spec/engines/memory/integration/joins/cross_engine_spec.rb b/spec/engines/memory/integration/joins/cross_engine_spec.rb
index 606f3154c7..5dc1a6cb99 100644
--- a/spec/engines/memory/integration/joins/cross_engine_spec.rb
+++ b/spec/engines/memory/integration/joins/cross_engine_spec.rb
@@ -13,10 +13,9 @@ module Arel
       @photos.insert(@photos[:id] => 1, @photos[:user_id] => 1, @photos[:camera_id] => 6)
       @photos.insert(@photos[:id] => 2, @photos[:user_id] => 2, @photos[:camera_id] => 42)
       # Oracle adapter returns database integers as Ruby integers and not strings
-      @adapter_returns_integer = false
-      adapter_is :oracle do
-        @adapter_returns_integer = true
-      end
+      # So does the FFI sqlite library
+      db_int_return = @photos.project(@photos[:camera_id]).first.tuple.first
+      @adapter_returns_integer = db_int_return.is_a?(String) ? false : true
     end
 
     describe 'when the in memory relation is on the left' do
diff --git a/spec/engines/sql/unit/predicates/in_spec.rb b/spec/engines/sql/unit/predicates/in_spec.rb
index f62ee6e829..be311f9f83 100644
--- a/spec/engines/sql/unit/predicates/in_spec.rb
+++ b/spec/engines/sql/unit/predicates/in_spec.rb
@@ -137,11 +137,7 @@ module Arel
             end
 
             adapter_is :sqlite3 do
-              if RUBY_VERSION < '1.9'
-                sql.should be_like(%Q{"developers"."created_at" BETWEEN '2010-01-01 00:00:00.000000' AND '2010-02-01 00:00:00.000000'})
-              else
-                sql.should be_like(%Q{"developers"."created_at" BETWEEN '2010-01-01 00:00:00' AND '2010-02-01 00:00:00'})
-              end
+              sql.should match(/"developers"."created_at" BETWEEN '2010-01-01 00:00:00(?:\.\d+)' AND '2010-02-01 00:00:00(?:\.\d+)'/)
             end
 
             adapter_is :postgresql do
diff --git a/spec/engines/sql/unit/relations/having_spec.rb b/spec/engines/sql/unit/relations/having_spec.rb
index fe6f3cc520..a7f2f0da96 100644
--- a/spec/engines/sql/unit/relations/having_spec.rb
+++ b/spec/engines/sql/unit/relations/having_spec.rb
@@ -39,6 +39,39 @@ module Arel
           end
         end
       end
+      
+      describe 'when given two predicates' do
+        it "manufactures sql with where clause conditions joined by AND" do
+          sql = @relation.group(@relation[:department]).having("MIN(salary) > 1000", "MAX(salary) < 10000").to_sql
+
+          adapter_is :mysql do
+            sql.should be_like(%Q{
+              SELECT `developers`.`id`, `developers`.`name`, `developers`.`salary`, `developers`.`department`, `developers`.`created_at`
+              FROM `developers`
+              GROUP BY `developers`.`department`
+              HAVING MIN(salary) > 1000 AND MAX(salary) < 10000
+            })
+          end
+
+          adapter_is :oracle do
+            sql.should be_like(%Q{
+              SELECT "DEVELOPERS"."ID", "DEVELOPERS"."NAME", "DEVELOPERS"."SALARY", "DEVELOPERS"."DEPARTMENT", "DEVELOPERS"."CREATED_AT"
+              FROM "DEVELOPERS"
+              GROUP BY "DEVELOPERS"."DEPARTMENT"
+              HAVING MIN(salary) > 1000 AND MAX(salary) < 10000
+            })
+          end
+
+          adapter_is_not :mysql, :oracle do
+            sql.should be_like(%Q{
+              SELECT "developers"."id", "developers"."name", "developers"."salary", "developers"."department", "developers"."created_at"
+              FROM "developers"
+              GROUP BY "developers"."department"
+              HAVING MIN(salary) > 1000 AND MAX(salary) < 10000
+            })
+          end
+        end
+      end
     end
   end
 end
diff --git a/spec/relations/join_spec.rb b/spec/relations/join_spec.rb
index 3894d175e8..b5ead51053 100644
--- a/spec/relations/join_spec.rb
+++ b/spec/relations/join_spec.rb
@@ -29,7 +29,7 @@ describe "Arel" do
         8.times do |i|
           thing_id = owner_id * 8 + i
           age      = 2 * thing_id
-          name     = "Name#{thing_id}"
+          name     = "Name #{thing_id % 6}"
 
           @thing.insert([thing_id, owner_id, name, age])
           @expected << Arel::Row.new(@relation, [thing_id, owner_id, name, age, owner_id])
diff --git a/spec/relations/relation_spec.rb b/spec/relations/relation_spec.rb
index 0381f8759d..b5b73e2205 100644
--- a/spec/relations/relation_spec.rb
+++ b/spec/relations/relation_spec.rb
@@ -14,7 +14,7 @@ describe "Arel" do
 
   describe "Relation" do
     before :all do
-      @expected = (1..20).map { |i| @relation.insert([i, "Name#{i}", 2 * i]) }
+      @expected = (1..20).map { |i| @relation.insert([i, "Name #{i % 6}", 2 * i]) }
     end
 
     it_should_behave_like 'A Relation'
diff --git a/spec/shared/relation_spec.rb b/spec/shared/relation_spec.rb
index e0c74fc7ee..b49e79f13d 100644
--- a/spec/shared/relation_spec.rb
+++ b/spec/shared/relation_spec.rb
@@ -154,25 +154,56 @@ share_examples_for 'A Relation' do
   describe "#order" do
     describe "by one attribute" do
       before :all do
-        @expected.map! { |r| r[@relation[:age]] }
-        @expected.sort!
+        @expected.sort! { |a, b| a[@relation[:age]] <=> b[@relation[:age]]}.map! {|e| e[@relation[:id]]}
       end
 
       it "can be specified as ascending order" do
         actual = []
-        @relation.order(@relation[:age].asc).each { |r| actual << r[@relation[:age]] }
+        @relation.order(@relation[:age].asc).each { |r| actual << r[@relation[:id]] }
         actual.should == @expected
       end
 
       it "can be specified as descending order" do
         actual = []
-        @relation.order(@relation[:age].desc).each { |r| actual << r[@relation[:age]] }
+        @relation.order(@relation[:age].desc).each { |r| actual << r[@relation[:id]] }
         actual.should == @expected.reverse
       end
     end
 
-    describe "by two attributes" do
-      it "works"
+    describe "by two attributes in two separate calls to #order" do
+      before :all do
+        @expected = @expected.sort_by { |e| [e[@relation[:name]], e[@relation[:age]]]}.map {|e| e[@relation[:id]]}
+      end
+
+      it "can be specified as ascending order" do
+        actual = []
+        @relation.order(@relation[:age].asc).order(@relation[:name].asc).each { |r| actual << r[@relation[:id]] }
+        actual.should == @expected
+      end
+
+      it "can be specified as descending order" do
+        actual = []
+        @relation.order(@relation[:age].desc).order(@relation[:name].desc).each { |r| actual << r[@relation[:id]] }
+        actual.should == @expected.reverse
+      end
+    end
+    
+    describe "by two attributes in one call to #order" do
+      before :all do
+        @expected = @expected.sort_by { |e| [e[@relation[:name]], e[@relation[:age]]]}.map {|e| e[@relation[:id]]}
+      end
+      
+      it "can be specified as ascending order in one call to #order" do
+        actual = []
+        @relation.order(@relation[:name].asc, @relation[:age].asc).each { |r| actual << r[@relation[:id]] }
+        actual.should == @expected
+      end
+
+      it "can be specified as descending order in one call to #order" do
+        actual = []
+        @relation.order(@relation[:name].desc, @relation[:age].desc).each { |r| actual << r[@relation[:id]] }
+        actual.should == @expected.reverse
+      end
     end
   end