Merge pull request #39881 from kamipo/arel_attribute_normalization

Move Arel attribute normalization into `arel_table`

activerecord/lib/active_record/associations/alias_tracker.rb

@@ -51,11 +51,11 @@ module ActiveRecord
         @connection = connection
       end
 
-      def aliased_table_for(table_name, aliased_name, type_caster)
+      def aliased_table_for(table_name, aliased_name, klass)
         if aliases[table_name].zero?
           # If it's zero, we can have our table_name
           aliases[table_name] = 1
-          Arel::Table.new(table_name, type_caster: type_caster)
+          Arel::Table.new(table_name, klass: klass)
         else
           # Otherwise, we need to use an alias
           aliased_name = @connection.table_alias_for(aliased_name)
@@ -68,7 +68,7 @@ module ActiveRecord
           else
             aliased_name
           end
-          Arel::Table.new(table_name, type_caster: type_caster).alias(table_alias)
+          Arel::Table.new(table_name, klass: klass).alias(table_alias)
         end
       end
 

activerecord/lib/active_record/associations/association_scope.rb

@@ -109,7 +109,7 @@ module ActiveRecord
             aliased_table = tracker.aliased_table_for(
               refl.table_name,
               refl.alias_candidate(name),
-              refl.klass.type_caster
+              refl.klass
             )
             chain << ReflectionProxy.new(refl, aliased_table)
           end

activerecord/lib/active_record/associations/join_dependency.rb

@@ -176,7 +176,7 @@ module ActiveRecord
             alias_tracker.aliased_table_for(
               reflection.table_name,
               table_alias_for(reflection, parent, reflection != child.reflection),
-              reflection.klass.type_caster
+              reflection.klass
             )
           end.concat child.children.flat_map { |c| make_constraints(child, c, join_type) }
         end

activerecord/lib/active_record/core.rb

@@ -289,14 +289,13 @@ module ActiveRecord
       #     scope :published_and_commented, -> { published.and(arel_table[:comments_count].gt(0)) }
       #   end
       def arel_table # :nodoc:
-        @arel_table ||= Arel::Table.new(table_name, type_caster: type_caster)
+        @arel_table ||= Arel::Table.new(table_name, klass: self)
       end
 
       def arel_attribute(name, table = arel_table) # :nodoc:
-        name = name.to_s
-        name = attribute_aliases[name] || name
         table[name]
       end
+      deprecate :arel_attribute
 
       def predicate_builder # :nodoc:
         @predicate_builder ||= PredicateBuilder.new(table_metadata)

activerecord/lib/active_record/inheritance.rb

@@ -260,7 +260,7 @@ module ActiveRecord
         end
 
         def type_condition(table = arel_table)
-          sti_column = arel_attribute(inheritance_column, table)
+          sti_column = table[inheritance_column]
           sti_names  = ([self] + descendants).map(&:sti_name)
 
           predicate_builder.build(sti_column, sti_names)

activerecord/lib/active_record/persistence.rb

@@ -414,7 +414,7 @@ module ActiveRecord
 
         def _substitute_values(values)
           values.map do |name, value|
-            attr = arel_attribute(name)
+            attr = arel_table[name]
             bind = predicate_builder.build_bind_attribute(name, value)
             [attr, bind]
           end

activerecord/lib/active_record/reflection.rb

@@ -1052,7 +1052,7 @@ module ActiveRecord
       end
 
       def aliased_table
-        @aliased_table ||= Arel::Table.new(table_name, type_caster: klass.type_caster)
+        @aliased_table ||= Arel::Table.new(table_name, klass: klass)
       end
 
       def join_primary_key(klass = self.klass)

activerecord/lib/active_record/relation.rb

@@ -39,8 +39,9 @@ module ActiveRecord
     end
 
     def arel_attribute(name) # :nodoc:
-      klass.arel_attribute(name, table)
+      table[name]
     end
+    deprecate :arel_attribute
 
     def bind_attribute(name, value) # :nodoc:
       if reflection = klass._reflect_on_association(name)
@@ -48,7 +49,7 @@ module ActiveRecord
         value = value.read_attribute(reflection.klass.primary_key) unless value.nil?
       end
 
-      attr = arel_attribute(name)
+      attr = table[name]
       bind = predicate_builder.build_bind_attribute(attr.name, value)
       yield attr, bind
     end
@@ -352,7 +353,7 @@ module ActiveRecord
       else
         collection = eager_loading? ? apply_join_dependency : self
 
-        column = connection.visitor.compile(arel_attribute(timestamp_column))
+        column = connection.visitor.compile(table[timestamp_column])
         select_values = "COUNT(*) AS #{connection.quote_column_name("size")}, MAX(%s) AS timestamp"
 
         if collection.has_limit_or_offset?
@@ -447,7 +448,7 @@ module ActiveRecord
 
       stmt = Arel::UpdateManager.new
       stmt.table(arel.join_sources.empty? ? table : arel.source)
-      stmt.key = arel_attribute(primary_key)
+      stmt.key = table[primary_key]
       stmt.take(arel.limit)
       stmt.offset(arel.offset)
       stmt.order(*arel.orders)
@@ -457,7 +458,7 @@ module ActiveRecord
         if klass.locking_enabled? &&
             !updates.key?(klass.locking_column) &&
             !updates.key?(klass.locking_column.to_sym)
-          attr = arel_attribute(klass.locking_column)
+          attr = table[klass.locking_column]
           updates[attr.name] = _increment_attribute(attr)
         end
         stmt.set _substitute_values(updates)
@@ -493,7 +494,7 @@ module ActiveRecord
 
       updates = {}
       counters.each do |counter_name, value|
-        attr = arel_attribute(counter_name)
+        attr = table[counter_name]
         updates[attr.name] = _increment_attribute(attr, value)
       end
 
@@ -589,7 +590,7 @@ module ActiveRecord
 
       stmt = Arel::DeleteManager.new
       stmt.from(arel.join_sources.empty? ? table : arel.source)
-      stmt.key = arel_attribute(primary_key)
+      stmt.key = table[primary_key]
       stmt.take(arel.limit)
       stmt.offset(arel.offset)
       stmt.order(*arel.orders)
@@ -813,7 +814,7 @@ module ActiveRecord
 
       def _substitute_values(values)
         values.map do |name, value|
-          attr = arel_attribute(name)
+          attr = table[name]
           unless Arel.arel_node?(value)
             type = klass.type_for_attribute(attr.name)
             value = predicate_builder.build_bind_attribute(attr.name, type.cast(value))

activerecord/lib/active_record/relation/batches.rb

@@ -280,7 +280,7 @@ module ActiveRecord
       end
 
       def batch_order(order)
-        arel_attribute(primary_key).public_send(order)
+        table[primary_key].public_send(order)
       end
 
       def act_on_ignored_order(error_on_ignore)

activerecord/lib/active_record/relation/finder_methods.rb

@@ -410,7 +410,7 @@ module ActiveRecord
 
       def limited_ids_for(relation)
         values = @klass.connection.columns_for_distinct(
-          connection.visitor.compile(arel_attribute(primary_key)),
+          connection.visitor.compile(table[primary_key]),
           relation.order_values
         )
 
@@ -562,9 +562,9 @@ module ActiveRecord
       def ordered_relation
         if order_values.empty? && (implicit_order_column || primary_key)
           if implicit_order_column && primary_key && implicit_order_column != primary_key
-            order(arel_attribute(implicit_order_column).asc, arel_attribute(primary_key).asc)
+            order(table[implicit_order_column].asc, table[primary_key].asc)
           else
-            order(arel_attribute(implicit_order_column || primary_key).asc)
+            order(table[implicit_order_column || primary_key].asc)
           end
         else
           self

activerecord/lib/active_record/relation/predicate_builder.rb

@@ -70,7 +70,7 @@ module ActiveRecord
     end
 
     def resolve_arel_attribute(table_name, column_name, &block)
-      table.associated_table(table_name, &block).arel_attribute(column_name)
+      table.associated_table(table_name, &block).arel_table[column_name]
     end
 
     protected
@@ -114,18 +114,18 @@ module ActiveRecord
               values = values.map do |object|
                 object.respond_to?(aggr_attr) ? object.public_send(aggr_attr) : object
               end
-              build(table.arel_attribute(column_name), values)
+              build(table.arel_table[column_name], values)
             else
               queries = values.map do |object|
                 mapping.map do |field_attr, aggregate_attr|
-                  build(table.arel_attribute(field_attr), object.try!(aggregate_attr))
+                  build(table.arel_table[field_attr], object.try!(aggregate_attr))
                 end
               end
 
               grouping_queries(queries)
             end
           else
-            build(table.arel_attribute(key), value)
+            build(table.arel_table[key], value)
           end
         end
       end

activerecord/lib/active_record/relation/predicate_builder/relation_handler.rb

@@ -9,7 +9,7 @@ module ActiveRecord
         end
 
         if value.select_values.empty?
-          value = value.select(value.arel_attribute(value.klass.primary_key))
+          value = value.select(value.table[value.klass.primary_key])
         end
 
         attribute.in(value.arel)

activerecord/lib/active_record/relation/query_methods.rb

@@ -1304,7 +1304,7 @@ module ActiveRecord
         if select_values.any?
           arel.project(*arel_columns(select_values.uniq))
         elsif klass.ignored_columns.any?
-          arel.project(*klass.column_names.map { |field| arel_attribute(field) })
+          arel.project(*klass.column_names.map { |field| table[field] })
         else
           arel.project(table[Arel.star])
         end
@@ -1332,7 +1332,7 @@ module ActiveRecord
         from = from_clause.name || from_clause.value
 
         if klass.columns_hash.key?(field) && (!from || table_name_matches?(from))
-          arel_attribute(field)
+          table[field]
         elsif field.match?(/\A\w+\.\w+\z/)
           table, column = field.split(".")
           predicate_builder.resolve_arel_attribute(table, column) do
@@ -1351,7 +1351,7 @@ module ActiveRecord
 
       def reverse_sql_order(order_query)
         if order_query.empty?
-          return [arel_attribute(primary_key).desc] if primary_key
+          return [table[primary_key].desc] if primary_key
           raise IrreversibleOrderError,
             "Relation has no current order and table has no primary key to be used as default order"
         end
@@ -1457,7 +1457,7 @@ module ActiveRecord
       def order_column(field)
         arel_column(field) do |attr_name|
           if attr_name == "count" && !group_values.empty?
-            arel_attribute(attr_name)
+            table[attr_name]
           else
             Arel.sql(connection.quote_table_name(attr_name))
           end

activerecord/lib/active_record/table_metadata.rb

@@ -10,14 +10,6 @@ module ActiveRecord
       @reflection = reflection
     end
 
-    def arel_attribute(column_name)
-      if klass
-        klass.arel_attribute(column_name, arel_table)
-      else
-        arel_table[column_name]
-      end
-    end
-
     def type(column_name)
       arel_table.type_for_attribute(column_name)
     end
@@ -73,7 +65,9 @@ module ActiveRecord
       end
     end
 
+    attr_reader :arel_table
+
     private
-      attr_reader :klass, :arel_table, :reflection
+      attr_reader :klass, :reflection
   end
 end

activerecord/lib/arel/nodes/table_alias.rb

@@ -8,7 +8,7 @@ module Arel # :nodoc: all
       alias :table_alias :name
 
       def [](name)
-        Attribute.new(self, name)
+        relation.is_a?(Table) ? relation[name, self] : Attribute.new(self, name)
       end
 
       def table_name

activerecord/lib/arel/table.rb

@@ -14,8 +14,9 @@ module Arel # :nodoc: all
     # TableAlias and Table both have a #table_name which is the name of the underlying table
     alias :table_name :name
 
-    def initialize(name, as: nil, type_caster: nil)
+    def initialize(name, as: nil, klass: nil, type_caster: klass&.type_caster)
       @name = name.to_s
+      @klass = klass
       @type_caster = type_caster
 
       # Sometime AR sends an :as parameter to table, to let the table know
@@ -79,8 +80,10 @@ module Arel # :nodoc: all
       from.having expr
     end
 
-    def [](name)
-      ::Arel::Attribute.new self, name
+    def [](name, table = self)
+      name = name.to_s if name.is_a?(Symbol)
+      name = @klass.attribute_aliases[name] || name if @klass
+      Attribute.new(table, name)
     end
 
     def hash

activerecord/test/cases/adapters/postgresql/case_insensitive_test.rb

@@ -8,19 +8,19 @@ class PostgresqlCaseInsensitiveTest < ActiveRecord::PostgreSQLTestCase
   def test_case_insensitiveness
     connection = ActiveRecord::Base.connection
 
-    attr = Default.arel_attribute(:char1)
+    attr = Default.arel_table[:char1]
     comparison = connection.case_insensitive_comparison(attr, nil)
     assert_match(/lower/i, comparison.to_sql)
 
-    attr = Default.arel_attribute(:char2)
+    attr = Default.arel_table[:char2]
     comparison = connection.case_insensitive_comparison(attr, nil)
     assert_match(/lower/i, comparison.to_sql)
 
-    attr = Default.arel_attribute(:char3)
+    attr = Default.arel_table[:char3]
     comparison = connection.case_insensitive_comparison(attr, nil)
     assert_match(/lower/i, comparison.to_sql)
 
-    attr = Default.arel_attribute(:multiline_default)
+    attr = Default.arel_table[:multiline_default]
     comparison = connection.case_insensitive_comparison(attr, nil)
     assert_match(/lower/i, comparison.to_sql)
   end

activerecord/test/cases/arel/table_test.rb

@@ -188,7 +188,7 @@ module Arel
       describe "when given a Symbol" do
         it "manufactures an attribute if the symbol names an attribute within the relation" do
           column = @relation[:id]
-          _(column.name).must_equal :id
+          _(column.name).must_equal "id"
         end
       end
     end

activerecord/test/cases/associations/inner_join_association_test.rb

@@ -71,7 +71,7 @@ class InnerJoinAssociationTest < ActiveRecord::TestCase
 
   def test_deduplicate_joins
     posts = Post.arel_table
-    constraint = posts[:author_id].eq(Author.arel_attribute(:id))
+    constraint = posts[:author_id].eq(Author.arel_table[:id])
 
     authors = Author.joins(posts.create_join(posts, posts.create_on(constraint)))
     authors = authors.joins(:author_address).merge(authors.where("posts.type": "SpecialPost"))

activerecord/test/cases/base_test.rb

@@ -78,6 +78,23 @@ class BasicsTest < ActiveRecord::TestCase
     assert_equal "Post::GeneratedRelationMethods", mod.inspect
   end
 
+  def test_arel_attribute_normalization
+    assert_equal Post.arel_table["body"], Post.arel_table[:body]
+    assert_equal Post.arel_table["body"], Post.arel_table[:text]
+  end
+
+  def test_deprecated_arel_attribute
+    assert_deprecated do
+      assert_equal Post.arel_table["body"], Post.arel_attribute(:body)
+    end
+  end
+
+  def test_deprecated_arel_attribute_on_relation
+    assert_deprecated do
+      assert_equal Post.arel_table["body"], Post.all.arel_attribute(:body)
+    end
+  end
+
   def test_incomplete_schema_loading
     topic = Topic.first
     payload = { foo: 42 }

activerecord/test/cases/relations_test.rb

@@ -1253,7 +1253,7 @@ class RelationTest < ActiveRecord::TestCase
     assert_predicate same_parrot, :persisted?
     assert_equal parrot, same_parrot
 
-    canary = Bird.where(Bird.arel_attribute(:color).is_distinct_from("green")).first_or_create(name: "canary")
+    canary = Bird.where(Bird.arel_table[:color].is_distinct_from("green")).first_or_create(name: "canary")
     assert_equal "canary", canary.name
     assert_nil canary.color
   end
@@ -1377,7 +1377,7 @@ class RelationTest < ActiveRecord::TestCase
     assert_equal "parrot", parrot.name
     assert_equal "green", parrot.color
 
-    canary = Bird.where(Bird.arel_attribute(:color).is_distinct_from("green")).first_or_initialize(name: "canary")
+    canary = Bird.where(Bird.arel_table[:color].is_distinct_from("green")).first_or_initialize(name: "canary")
     assert_equal "canary", canary.name
     assert_nil canary.color
   end
@@ -1955,7 +1955,7 @@ class RelationTest < ActiveRecord::TestCase
     assert_equal post, custom_post_relation.joins(:author).where!(title: post.title).take
   end
 
-  test "arel_attribute respects a custom table" do
+  test "arel_table respects a custom table" do
     assert_equal [posts(:sti_comments)], custom_post_relation.ranked_by_comments.limit_by(1).to_a
   end
 
@@ -2085,7 +2085,7 @@ class RelationTest < ActiveRecord::TestCase
   end
 
   def test_unscope_with_arel_sql
-    posts = Post.where(Arel.sql("'Welcome to the weblog'").eq(Post.arel_attribute(:title)))
+    posts = Post.where(Arel.sql("'Welcome to the weblog'").eq(Post.arel_table[:title]))
 
     assert_equal 1, posts.count
     assert_equal Post.count, posts.unscope(where: :title).count

activerecord/test/models/post.rb

@@ -28,7 +28,7 @@ class Post < ActiveRecord::Base
 
   scope :containing_the_letter_a, -> { where("body LIKE '%a%'") }
   scope :titled_with_an_apostrophe, -> { where("title LIKE '%''%'") }
-  scope :ranked_by_comments, -> { order(arel_attribute(:comments_count).desc) }
+  scope :ranked_by_comments, -> { order(table[:comments_count].desc) }
 
   scope :limit_by, lambda { |l| limit(l) }
   scope :locked, -> { lock }
@@ -339,10 +339,6 @@ class FakeKlass
       sql
     end
 
-    def arel_attribute(name, table)
-      table[name]
-    end
-
     def disallow_raw_sql!(*args)
       # noop
     end