Undo performances regressions I introduced in bbe0a507f287c20ab4ae8a244fbfc810665deda5 and add test for an edge case. Add comments to explain the intent of the code.

2 files changed, 34 insertions, 5 deletions

diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb
index dd3f4699f7..cd6c4002d2 100644
--- a/activerecord/lib/active_record/attribute_methods/read.rb
+++ b/activerecord/lib/active_record/attribute_methods/read.rb
@@ -43,7 +43,7 @@ module ActiveRecord
             end
 
             if attr_name == primary_key && attr_name != "id"
-              define_read_method(:id, attr_name, columns_hash[attr_name])
+              define_read_method('id', attr_name, columns_hash[attr_name])
             end
           end
 
@@ -59,7 +59,9 @@ module ActiveRecord
           end
 
           # Define an attribute reader method.  Cope with nil column.
-          def define_read_method(symbol, attr_name, column)
+          # method_name is the same as attr_name except when a non-standard primary key is used,
+          # we still define #id as an accessor for the key
+          def define_read_method(method_name, attr_name, column)
             cast_code = column.type_cast_code('v')
             access_code = "(v=@attributes['#{attr_name}']) && #{cast_code}"
 
@@ -71,9 +73,25 @@ module ActiveRecord
               access_code = "@attributes_cache['#{attr_name}'] ||= (#{access_code})"
             end
 
-            generated_attribute_methods.module_eval do
-              define_method("_#{attr_name}") { eval(access_code) }
-              alias_method(attr_name, "_#{attr_name}")
+            # Where possible, generate the method by evalling a string, as this will result in
+            # faster accesses because it avoids the block eval and then string eval incurred
+            # by the second branch.
+            #
+            # The second, slower, branch is necessary to support instances where the database
+            # returns columns with extra stuff in (like 'my_column(omg)').
+            if method_name =~ /^[a-zA-Z_]\w*[!?=]?$/
+              generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__
+                def _#{method_name}
+                  #{access_code}
+                end
+
+                alias #{method_name} _#{method_name}
+              STR
+            else
+              generated_attribute_methods.module_eval do
+                define_method("_#{method_name}") { eval(access_code) }
+                alias_method(method_name, "_#{method_name}")
+              end
             end
           end
       end
diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb
index 899166d129..5074ae50ab 100644
--- a/activerecord/test/cases/attribute_methods_test.rb
+++ b/activerecord/test/cases/attribute_methods_test.rb
@@ -10,6 +10,7 @@ require 'models/company'
 require 'models/category'
 require 'models/reply'
 require 'models/contact'
+require 'models/keyboard'
 
 class AttributeMethodsTest < ActiveRecord::TestCase
   fixtures :topics, :developers, :companies, :computers
@@ -89,6 +90,16 @@ class AttributeMethodsTest < ActiveRecord::TestCase
     assert !topic.respond_to?(:nothingness)
   end
 
+  def test_respond_to_with_custom_primary_key
+    keyboard = Keyboard.create
+    assert_not_nil keyboard.key_number
+    assert_equal keyboard.key_number, keyboard.id
+    assert keyboard.respond_to?('key_number')
+    assert keyboard.respond_to?('_key_number')
+    assert keyboard.respond_to?('id')
+    assert keyboard.respond_to?('_id')
+  end
+
   # Syck calls respond_to? before actually calling initialize
   def test_respond_to_with_allocated_object
     topic = Topic.allocate