Added DISTINCT support. Modified when to quote or not columns and tables.

7 files changed, 54 insertions, 24 deletions

diff --git a/lib/arel/primitives/attribute.rb b/lib/arel/primitives/attribute.rb
index 7021e9f9ed..6cb558d8ce 100644
--- a/lib/arel/primitives/attribute.rb
+++ b/lib/arel/primitives/attribute.rb
@@ -120,8 +120,8 @@ module Arel
     include Predications
 
     module Expressions
-      def count
-        Expression.new(self, "COUNT")
+      def count(distinct = false)
+        distinct ?  Expression.new(self, "DISTINCT").count :  Expression.new(self, "COUNT")
       end
 
       def sum
diff --git a/lib/arel/primitives/value.rb b/lib/arel/primitives/value.rb
index 36a7fb7c71..9c6e518a95 100644
--- a/lib/arel/primitives/value.rb
+++ b/lib/arel/primitives/value.rb
@@ -6,7 +6,11 @@ module Arel
 
 
     def to_sql(formatter = Sql::WhereCondition.new(relation))
-      formatter.value value
+      if value =~ /^\(.*\)$/
+        value
+      else
+        formatter.value value
+      end
     end
 
     def format(object)
diff --git a/lib/arel/relations/relation.rb b/lib/arel/relations/relation.rb
index 986bc3fbeb..50c46aa2ed 100644
--- a/lib/arel/relations/relation.rb
+++ b/lib/arel/relations/relation.rb
@@ -1,5 +1,7 @@
 module Arel
   class Relation
+    attr_reader :count
+
     def session
       Session.new
     end
@@ -12,6 +14,11 @@ module Arel
       engine.select_values self.to_sql
     end
 
+    def count
+      @count = "COUNT(*) AS count_all"
+      engine.select_value self.to_sql
+    end
+
     def to_sql(formatter = Sql::SelectStatement.new(self))
       formatter.select select_sql, self
     end
@@ -19,7 +26,7 @@ module Arel
 
     def select_sql
       [
-        "SELECT     #{attributes.collect { |a| a.to_sql(Sql::SelectClause.new(self)) }.join(', ')}",
+        "SELECT  #{@count} #{attributes.collect { |a| a.to_sql(Sql::SelectClause.new(self)) }.join(', ') unless @count}",
         "FROM       #{table_sql(Sql::TableReference.new(self))}",
         (joins(self)                                                                                    unless joins(self).blank? ),
         ("WHERE     #{wheres   .collect { |w| w.to_sql(Sql::WhereClause.new(self)) }.join("\n\tAND ")}" unless wheres.blank?      ),
diff --git a/lib/arel/sql/formatters.rb b/lib/arel/sql/formatters.rb
index f105fbea72..aa10952d04 100644
--- a/lib/arel/sql/formatters.rb
+++ b/lib/arel/sql/formatters.rb
@@ -15,14 +15,18 @@ module Arel
       def attribute(attribute)
         # FIXME this should check that the column exists
         if attribute.name.to_s =~ /^\w*$/
-        "#{quote_table_name(name_for(attribute.original_relation))}.#{quote_column_name(attribute.name)}" + (attribute.alias ? " AS #{quote(attribute.alias.to_s)}" : "")
+          "#{quote_table_name(name_for(attribute.original_relation))}.#{quote_column_name(attribute.name)}" + (attribute.alias ? " AS #{quote(attribute.alias.to_s)}" : "")
         else
           attribute.name.to_s + (attribute.alias ? " AS #{quote(attribute.alias.to_s)}" : "")
         end
       end
 
       def expression(expression)
-        "#{expression.function_sql}(#{expression.attribute.to_sql(self)})" + (expression.alias ? " AS #{quote_column_name(expression.alias)}" : '')
+        if expression.function_sql == "DISTINCT"
+          "#{expression.function_sql} #{expression.attribute.to_sql(self)}" + (expression.alias ? " AS #{quote_column_name(expression.alias)}" : '')
+        else
+          "#{expression.function_sql}(#{expression.attribute.to_sql(self)})" + (expression.alias ? " AS #{quote_column_name(expression.alias)}" : '')
+        end
       end
 
       def select(select_sql, table)
@@ -89,7 +93,11 @@ module Arel
       end
 
       def table(table)
-        quote_table_name(table.name) + (table.name != name_for(table) ? " AS " + quote_table_name(name_for(table)) : '')
+        if table.name =~ /^(\w|-)*$/
+          quote_table_name(table.name) + (table.name != name_for(table) ? " AS " + quote_table_name(name_for(table)) : '')
+        else
+          table.name + (table.name != name_for(table) ? " AS " + (name_for(table)) : '')
+        end
       end
     end
 
diff --git a/spec/arel/unit/primitives/expression_spec.rb b/spec/arel/unit/primitives/expression_spec.rb
index d398805fe2..4943f4ef33 100644
--- a/spec/arel/unit/primitives/expression_spec.rb
+++ b/spec/arel/unit/primitives/expression_spec.rb
@@ -6,40 +6,40 @@ module Arel
       @relation = Table.new(:users)
       @attribute = @relation[:id]
     end
-    
+
     describe Expression::Transformations do
       before do
         @expression = Expression.new(@attribute, "COUNT")
       end
-      
+
       describe '#bind' do
         it "manufactures an attribute with a rebound relation and self as the ancestor" do
           derived_relation = @relation.where(@relation[:id].eq(1))
           @expression.bind(derived_relation).should == Expression.new(@attribute.bind(derived_relation), "COUNT", nil, @expression)
         end
-        
+
         it "returns self if the substituting to the same relation" do
           @expression.bind(@relation).should == @expression
         end
       end
-      
+
       describe '#as' do
         it "manufactures an aliased expression" do
           @expression.as(:alias).should == Expression.new(@attribute, "COUNT", :alias, @expression)
         end
       end
-      
+
       describe '#to_attribute' do
         it "manufactures an attribute with the expression as an ancestor" do
           @expression.to_attribute.should == Attribute.new(@expression.relation, @expression.alias, :ancestor => @expression)
         end
       end
     end
-    
+
     describe '#to_sql' do
       it "manufactures sql with the expression and alias" do
         Expression.new(@attribute, "COUNT", :alias).to_sql.should == "COUNT(`users`.`id`) AS `alias`"
       end
     end
   end
-end
\ No newline at end of file
+end
diff --git a/spec/arel/unit/relations/project_spec.rb b/spec/arel/unit/relations/project_spec.rb
index 93cbe5668a..7f531096f0 100644
--- a/spec/arel/unit/relations/project_spec.rb
+++ b/spec/arel/unit/relations/project_spec.rb
@@ -6,17 +6,17 @@ module Arel
       @relation = Table.new(:users)
       @attribute = @relation[:id]
     end
-    
+
     describe '#attributes' do
       before do
         @projection = Project.new(@relation, @attribute)
       end
-      
+
       it "manufactures attributes associated with the projection relation" do
         @projection.attributes.should == [@attribute].collect { |a| a.bind(@projection) }
       end
     end
-  
+
     describe '#to_sql' do
       describe 'when given an attribute' do
         it "manufactures sql with a limited select clause" do
@@ -26,19 +26,19 @@ module Arel
           ")
         end
       end
-      
+
       describe 'when given a relation' do
         before do
           @scalar_relation = Project.new(@relation, @relation[:name])
         end
-        
+
         it "manufactures sql with scalar selects" do
           Project.new(@relation, @scalar_relation).to_sql.should be_like("
             SELECT (SELECT `users`.`name` FROM `users`) AS `users` FROM `users`
           ")
         end
       end
-      
+
       describe 'when given a string' do
         it "passes the string through to the select clause" do
           Project.new(@relation, 'asdf').to_sql.should be_like("
@@ -46,7 +46,7 @@ module Arel
           ")
         end
       end
-      
+
       describe 'when given an expression' do
         it 'manufactures sql with expressions' do
           @relation.project(@attribute.count).to_sql.should be_like("
@@ -54,16 +54,23 @@ module Arel
             FROM `users`
           ")
         end
+
+        it 'manufactures sql with distinct expressions' do
+          @relation.project(@attribute.count(true)).to_sql.should be_like("
+            SELECT COUNT(DISTINCT `users`.`id`)
+            FROM `users`
+          ")
+        end
       end
     end
-    
+
     describe '#externalizable?' do
       describe 'when the projections are attributes' do
         it 'returns false' do
           Project.new(@relation, @attribute).should_not be_externalizable
         end
       end
-      
+
       describe 'when the projections include an aggregation' do
         it "obtains" do
           Project.new(@relation, @attribute.sum).should be_externalizable
@@ -71,4 +78,4 @@ module Arel
       end
     end
   end
-end
\ No newline at end of file
+end
diff --git a/spec/doubles/database.rb b/spec/doubles/database.rb
index 7670958d7a..f8a4b38e17 100644
--- a/spec/doubles/database.rb
+++ b/spec/doubles/database.rb
@@ -34,6 +34,10 @@ module Fake
     def quote_table_name(table_name)
       "`#{table_name}`"
     end
+
+    def supports_count_distinct?
+      true
+    end
   end
 
   class Column