Merge pull request #38719 from eileencodes/refactor-fetch_attribute

Refactor fetch_attribute

activerecord/lib/arel.rb

@@ -47,16 +47,8 @@ def self.arel_node?(value) # :nodoc:
   end
 
   def self.fetch_attribute(value, &block) # :nodoc:
-    case value
-    when Arel::Nodes::Between, Arel::Nodes::In, Arel::Nodes::NotIn, Arel::Nodes::Equality,
-         Arel::Nodes::NotEqual, Arel::Nodes::LessThan, Arel::Nodes::LessThanOrEqual,
-         Arel::Nodes::GreaterThan, Arel::Nodes::GreaterThanOrEqual
-      attribute_value = value.detect_attribute
-      yield attribute_value if attribute_value
-    when Arel::Nodes::Or
-      fetch_attribute(value.left, &block) && fetch_attribute(value.right, &block)
-    when Arel::Nodes::Grouping
-      fetch_attribute(value.expr, &block)
+    unless String === value
+      value.fetch_attribute(&block)
     end
   end
 end

activerecord/lib/arel/nodes/binary.rb

@@ -11,14 +11,6 @@ def initialize(left, right)
         @right = right
       end
 
-      def detect_attribute
-        if self.left.is_a?(Arel::Attributes::Attribute)
-          self.left
-        elsif self.right.is_a?(Arel::Attributes::Attribute)
-          self.right
-        end
-      end
-
       def initialize_copy(other)
         super
         @left  = @left.clone if @left
@@ -37,18 +29,34 @@ def eql?(other)
       alias :== :eql?
     end
 
+    module FetchAttribute
+      def fetch_attribute
+        if left.is_a?(Arel::Attributes::Attribute)
+          yield left
+        elsif right.is_a?(Arel::Attributes::Attribute)
+          yield right
+        end
+      end
+    end
+
+    class Between < Binary; include FetchAttribute; end
+    class NotIn < Binary; include FetchAttribute; end
+    class GreaterThan < Binary; include FetchAttribute; end
+    class GreaterThanOrEqual < Binary; include FetchAttribute; end
+    class NotEqual < Binary; include FetchAttribute; end
+    class LessThan < Binary; include FetchAttribute; end
+    class LessThanOrEqual < Binary; include FetchAttribute; end
+
+    class Or < Binary
+      def fetch_attribute(&block)
+        left.fetch_attribute(&block) && right.fetch_attribute(&block)
+      end
+    end
+
     %w{
       As
       Assignment
-      Between
-      GreaterThan
-      GreaterThanOrEqual
       Join
-      LessThan
-      LessThanOrEqual
-      NotEqual
-      NotIn
-      Or
       Union
       UnionAll
       Intersect

activerecord/lib/arel/nodes/equality.rb

@@ -10,6 +10,14 @@ def operator; :== end
       def invert
         Arel::Nodes::NotEqual.new(left, right)
       end
+
+      def fetch_attribute
+        if left.is_a?(Arel::Attributes::Attribute)
+          yield left
+        elsif right.is_a?(Arel::Attributes::Attribute)
+          yield right
+        end
+      end
     end
 
     class IsDistinctFrom < Equality

activerecord/lib/arel/nodes/grouping.rb

@@ -3,6 +3,9 @@
 module Arel # :nodoc: all
   module Nodes
     class Grouping < Unary
+      def fetch_attribute(&block)
+        expr.fetch_attribute(&block)
+      end
     end
   end
 end

activerecord/lib/arel/nodes/node.rb

@@ -41,6 +41,9 @@ def to_sql(engine = Table.engine)
         collector = engine.connection.visitor.accept self, collector
         collector.value
       end
+
+      def fetch_attribute
+      end
     end
   end
 end

activerecord/lib/arel/nodes/sql_literal.rb

@@ -11,6 +11,9 @@ class SqlLiteral < String
       def encode_with(coder)
         coder.scalar = self.to_s
       end
+
+      def fetch_attribute
+      end
     end
   end
 end