From ec083687a96af1f35f9fb9e75611cc4bf4f5bf81 Mon Sep 17 00:00:00 2001
From: Sean Griffin <sean@thoughtbot.com>
Date: Wed, 26 Nov 2014 15:00:05 -0700
Subject: [PATCH] Remove deprecated method "Table#primary_key"

The only place this method was still used is on the MSSQL visitor. The
visitor has all of the objects required to inline this lookup there.
Since the `primary_key` method on the connection adapter will perform a
query when called, we can cache the result on the visitor.
---
 lib/arel/table.rb           | 14 --------------
 lib/arel/visitors/mssql.rb  | 21 ++++++++++++++++++---
 test/visitors/test_mssql.rb | 19 +++++++++++++++++++
 3 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/lib/arel/table.rb b/lib/arel/table.rb
index 01d4561ff1..fb86c03404 100644
--- a/lib/arel/table.rb
+++ b/lib/arel/table.rb
@@ -17,7 +17,6 @@ module Arel
       @columns = nil
       @aliases = []
       @table_alias = nil
-      @primary_key = nil
 
       if Hash === engine
         @engine  = engine[:engine] || Table.engine
@@ -29,19 +28,6 @@ module Arel
       end
     end
 
-    def primary_key
-      if $VERBOSE
-        warn <<-eowarn
-primary_key (#{caller.first}) is deprecated and will be removed in Arel 4.0.0
-        eowarn
-      end
-      @primary_key ||= begin
-        primary_key_name = @engine.connection.primary_key(name)
-        # some tables might be without primary key
-        primary_key_name && self[primary_key_name]
-      end
-    end
-
     def alias name = "#{self.name}_2"
       Nodes::TableAlias.new(self, name).tap do |node|
         @aliases << node
diff --git a/lib/arel/visitors/mssql.rb b/lib/arel/visitors/mssql.rb
index 0e5b75ec59..7c65ad33f2 100644
--- a/lib/arel/visitors/mssql.rb
+++ b/lib/arel/visitors/mssql.rb
@@ -3,6 +3,11 @@ module Arel
     class MSSQL < Arel::Visitors::ToSql
       RowNumber = Struct.new :children
 
+      def initialize(*)
+        @primary_keys = {}
+        super
+      end
+
       private
 
       # `top` wouldn't really work here. I.e. User.select("distinct first_name").limit(10) would generate
@@ -81,10 +86,20 @@ module Arel
       end
 
       # FIXME raise exception of there is no pk?
-      # FIXME!! Table.primary_key will be deprecated. What is the replacement??
       def find_left_table_pk o
-        return o.primary_key if o.instance_of? Arel::Table
-        find_left_table_pk o.left if o.kind_of? Arel::Nodes::Join
+        if o.kind_of?(Arel::Nodes::Join)
+          find_left_table_pk(o.left)
+        elsif o.instance_of?(Arel::Table)
+          find_primary_key(o)
+        end
+      end
+
+      def find_primary_key(o)
+        @primary_keys[o.name] ||= begin
+          primary_key_name = @connection.primary_key(o.name)
+          # some tables might be without primary key
+          primary_key_name && o[primary_key_name]
+        end
       end
     end
   end
diff --git a/test/visitors/test_mssql.rb b/test/visitors/test_mssql.rb
index a3efcb8b27..7574aeb0a2 100644
--- a/test/visitors/test_mssql.rb
+++ b/test/visitors/test_mssql.rb
@@ -26,6 +26,25 @@ module Arel
         sql.must_be_like "SELECT _t.* FROM (SELECT ROW_NUMBER() OVER (ORDER BY \"users\".\"id\") as _row_num FROM \"users\") as _t WHERE _row_num BETWEEN 1 AND 10"
       end
 
+      it 'caches the PK lookup for order' do
+        connection = MiniTest::Mock.new
+        connection.expect(:primary_key, ["id"], ["users"])
+
+        # We don't care how many times these methods are called
+        def connection.quote_table_name(*); ""; end
+        def connection.quote_column_name(*); ""; end
+
+        @visitor = MSSQL.new(connection)
+        stmt = Nodes::SelectStatement.new
+        stmt.cores.first.from = @table
+        stmt.limit = Nodes::Limit.new(10)
+
+        compile(stmt)
+        compile(stmt)
+
+        connection.verify
+      end
+
       it 'should go over query ORDER BY if .order()' do
         stmt = Nodes::SelectStatement.new
         stmt.limit = Nodes::Limit.new(10)