attributes do not need a column member

12 files changed, 65 insertions, 34 deletions

diff --git a/lib/arel/attributes/attribute.rb b/lib/arel/attributes/attribute.rb
index 5cbe194b41..9a42e5a4da 100644
--- a/lib/arel/attributes/attribute.rb
+++ b/lib/arel/attributes/attribute.rb
@@ -1,6 +1,6 @@
 module Arel
   module Attributes
-    class Attribute < Struct.new :relation, :name, :column
+    class Attribute < Struct.new :relation, :name
       include Arel::Expressions
       include Arel::Predications
     end
diff --git a/lib/arel/nodes/unqualified_column.rb b/lib/arel/nodes/unqualified_column.rb
index f7ba653c11..2820dba9d2 100644
--- a/lib/arel/nodes/unqualified_column.rb
+++ b/lib/arel/nodes/unqualified_column.rb
@@ -4,6 +4,10 @@ module Arel
       alias :attribute :expr
       alias :attribute= :expr=
 
+      def relation
+        @expr.relation
+      end
+
       def column
         @expr.column
       end
diff --git a/lib/arel/select_manager.rb b/lib/arel/select_manager.rb
index 08cfd41e7f..2bc7dbf1ec 100644
--- a/lib/arel/select_manager.rb
+++ b/lib/arel/select_manager.rb
@@ -143,8 +143,8 @@ module Arel
     def join_sql
       return nil unless @ctx.froms
 
-      viz = Visitors::JoinSql.new @engine
-      Nodes::SqlLiteral.new viz.accept @ctx
+      sql = @visitor.dup.extend(Visitors::JoinSql).accept @ctx
+      Nodes::SqlLiteral.new sql
     end
 
     def order_clauses
diff --git a/lib/arel/table.rb b/lib/arel/table.rb
index ca811debee..c2db388fd4 100644
--- a/lib/arel/table.rb
+++ b/lib/arel/table.rb
@@ -17,7 +17,6 @@ module Arel
 
       if Hash === engine
         @engine  = engine[:engine] || Table.engine
-        @columns = attributes_for engine[:columns]
 
         # Sometime AR sends an :as parameter to table, to let the table know
         # that it is an Alias.  We may want to override new, and return a
@@ -93,15 +92,18 @@ module Arel
     end
 
     def columns
+      if $VERBOSE
+        warn <<-eowarn
+(#{caller.first}) Arel::Table#columns is deprecated and will be removed in 
+Arel 2.2.0 with no replacement.
+        eowarn
+      end
       @columns ||=
         attributes_for @engine.connection.columns(@name, "#{@name} Columns")
     end
 
     def [] name
-      return nil unless table_exists?
-
-      name = name.to_sym
-      columns.find { |column| column.name == name }
+      ::Arel::Attribute.new self, name.to_sym
     end
 
     def select_manager
@@ -118,7 +120,7 @@ module Arel
       return nil unless columns
 
       columns.map do |column|
-        Attributes.for(column).new self, column.name.to_sym, column
+        Attributes.for(column).new self, column.name.to_sym
       end
     end
 
diff --git a/lib/arel/visitors/join_sql.rb b/lib/arel/visitors/join_sql.rb
index d3fb18d3c6..7ba2bde540 100644
--- a/lib/arel/visitors/join_sql.rb
+++ b/lib/arel/visitors/join_sql.rb
@@ -8,7 +8,7 @@ module Arel
     #
     # This visitor is used in SelectManager#join_sql and is for backwards
     # compatibility with Arel V1.0
-    class JoinSql < Arel::Visitors::ToSql
+    module JoinSql
       private
 
       def visit_Arel_Nodes_SelectCore o
diff --git a/lib/arel/visitors/to_sql.rb b/lib/arel/visitors/to_sql.rb
index b8a991b965..8db90c376a 100644
--- a/lib/arel/visitors/to_sql.rb
+++ b/lib/arel/visitors/to_sql.rb
@@ -10,7 +10,8 @@ module Arel
         @last_column    = nil
         @quoted_tables  = {}
         @quoted_columns = {}
-        @column_cache   = {}
+        @column_cache   = Hash.new { |h,k| h[k] = {} }
+        @table_exists   = {}
       end
 
       def accept object
@@ -67,17 +68,35 @@ module Arel
           o.alias ? " AS #{visit o.alias}" : ''}"
       end
 
-      def column_for relation, name
-        name    = name.to_s
-        table   = relation.name
+      def table_exists? name
+        return true if @table_exists.key? name
+        if @connection.table_exists?(name)
+          @table_exists[name] = true
+        else
+          false
+        end
+      end
+
+      def column_for attr
+        name    = attr.name.to_sym
+        table   = attr.relation.name
+
+        return nil unless table_exists? table
+
+        # If we don't have this column cached, get a list of columns and
+        # cache them for this table
+        unless @column_cache.key? table
+          #$stderr.puts "MISS: #{self.class.name} #{object_id} #{table.inspect} : #{name.inspect}"
+          columns = @connection.columns(table, "#{table}(#{name}) Columns")
+          @column_cache[table] = Hash[columns.map { |c| [c.name.to_sym, c] }]
+        end
 
-        columns = @connection.columns(table, "#{table} Columns")
-        columns.find { |col| col.name.to_s == name }
+        @column_cache[table][name]
       end
 
       def visit_Arel_Nodes_Values o
         "VALUES (#{o.expressions.zip(o.columns).map { |value, attr|
-          quote(value, attr && column_for(attr.relation, attr.name))
+          quote(value, attr && column_for(attr))
         }.join ', '})"
       end
 
@@ -235,7 +254,7 @@ module Arel
       end
 
       def visit_Arel_Nodes_Assignment o
-        right = quote(o.right, o.left.column)
+        right = quote(o.right, column_for(o.left))
         "#{visit o.left} = #{right}"
       end
 
@@ -268,7 +287,7 @@ module Arel
       end
 
       def visit_Arel_Attributes_Attribute o
-        @last_column = o.column
+        @last_column = column_for o
         join_name = o.relation.table_alias || o.relation.name
         "#{quote_table_name join_name}.#{quote_column_name o.name}"
       end
diff --git a/lib/arel/visitors/visitor.rb b/lib/arel/visitors/visitor.rb
index 055acf9765..85359f3e67 100644
--- a/lib/arel/visitors/visitor.rb
+++ b/lib/arel/visitors/visitor.rb
@@ -13,7 +13,8 @@ module Arel
 
       def visit object
         send DISPATCH[object.class], object
-      rescue NoMethodError
+      rescue NoMethodError => e
+        raise e if respond_to?(DISPATCH[object.class], true)
         warn "visiting #{object.class} via superclass, this will be removed in arel 2.2.0" if $VERBOSE
         superklass = object.class.ancestors.find { |klass|
           respond_to?(DISPATCH[klass], true)
diff --git a/test/attributes/test_attribute.rb b/test/attributes/test_attribute.rb
index 06954c242b..df7dc69621 100644
--- a/test/attributes/test_attribute.rb
+++ b/test/attributes/test_attribute.rb
@@ -326,7 +326,7 @@ module Arel
 
       describe '#eq' do
         it 'should return an equality node' do
-          attribute = Attribute.new nil, nil, nil
+          attribute = Attribute.new nil, nil
           equality = attribute.eq 1
           equality.left.must_equal attribute
           equality.right.must_equal 1
@@ -485,7 +485,7 @@ module Arel
         end
 
         it 'should return an in node' do
-          attribute = Attribute.new nil, nil, nil
+          attribute = Attribute.new nil, nil
           node = Nodes::In.new attribute, [1,2,3]
           node.left.must_equal attribute
           node.right.must_equal [1, 2, 3]
@@ -538,7 +538,7 @@ module Arel
         end
 
         it 'should return a NotIn node' do
-          attribute = Attribute.new nil, nil, nil
+          attribute = Attribute.new nil, nil
           node = Nodes::NotIn.new attribute, [1,2,3]
           node.left.must_equal attribute
           node.right.must_equal [1, 2, 3]
diff --git a/test/test_select_manager.rb b/test/test_select_manager.rb
index 0ecc78ce83..db2b306916 100644
--- a/test/test_select_manager.rb
+++ b/test/test_select_manager.rb
@@ -26,6 +26,13 @@ module Arel
     def quote_table_name thing; @engine.connection.quote_table_name thing end
     def quote_column_name thing; @engine.connection.quote_column_name thing end
     def quote thing, column; @engine.connection.quote thing, column end
+    def columns table, message = nil
+      @engine.connection.columns table, message
+    end
+
+    def table_exists? name
+      @engine.connection.table_exists? name
+    end
 
     def execute sql, name = nil, *args
       @executed << sql
diff --git a/test/test_table.rb b/test/test_table.rb
index 93cdde8f68..bb7bd255fd 100644
--- a/test/test_table.rb
+++ b/test/test_table.rb
@@ -174,13 +174,6 @@ module Arel
         it "manufactures an attribute if the symbol names an attribute within the relation" do
           column = @relation[:id]
           column.name.must_equal :id
-          column.must_be_kind_of Attributes::Integer
-        end
-      end
-
-      describe 'when table does not exist' do
-        it 'returns nil' do
-          @relation[:foooo].must_be_nil
         end
       end
     end
diff --git a/test/visitors/test_join_sql.rb b/test/visitors/test_join_sql.rb
index 8253fe5ab4..181ef6c570 100644
--- a/test/visitors/test_join_sql.rb
+++ b/test/visitors/test_join_sql.rb
@@ -4,7 +4,8 @@ module Arel
   module Visitors
     describe 'the join_sql visitor' do
       before do
-        @visitor = JoinSql.new Table.engine
+        @visitor = ToSql.new Table.engine
+        @visitor.extend(JoinSql)
       end
 
       describe 'inner join' do
diff --git a/test/visitors/test_to_sql.rb b/test/visitors/test_to_sql.rb
index 1c5c8eac0c..04d5e2d39f 100644
--- a/test/visitors/test_to_sql.rb
+++ b/test/visitors/test_to_sql.rb
@@ -84,7 +84,7 @@ module Arel
       end
 
       it "should visit visit_Arel_Attributes_Time" do
-        attr = Attributes::Time.new(@attr.relation, @attr.name, @attr.column)
+        attr = Attributes::Time.new(@attr.relation, @attr.name)
         @visitor.accept attr
       end
 
@@ -143,7 +143,9 @@ module Arel
           end
           in_node = Nodes::In.new @attr, %w{ a b c }
           visitor = visitor.new(Table.engine)
-          visitor.expected = @attr.column
+          visitor.expected = Table.engine.connection.columns(:users).find { |x|
+            x.name == 'name'
+          }
           visitor.accept(in_node).must_equal %("users"."name" IN ('a', 'b', 'c'))
         end
       end
@@ -189,7 +191,9 @@ module Arel
           end
           in_node = Nodes::NotIn.new @attr, %w{ a b c }
           visitor = visitor.new(Table.engine)
-          visitor.expected = @attr.column
+          visitor.expected = Table.engine.connection.columns(:users).find { |x|
+            x.name == 'name'
+          }
           visitor.accept(in_node).must_equal %("users"."name" NOT IN ('a', 'b', 'c'))
         end
       end