attributes do not need a column member

7 files changed, 46 insertions, 20 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)