Revert "primary_key now supports :limit. [#876 state:resolved]" since it broke AR test suite.

This reverts commit 41e5c7ed44fedb95636ef9b7a792c46ea03309bd.

5 files changed, 34 insertions, 57 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
index 5e29baf51f..64faaef4a0 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
@@ -274,7 +274,7 @@ module ActiveRecord
         column_options = {}
         column_options[:null] = null unless null.nil?
         column_options[:default] = default unless default.nil?
-        add_column_options!(column_sql, column_options)
+        add_column_options!(column_sql, column_options) unless type.to_sym == :primary_key
         column_sql
       end
 
@@ -334,8 +334,8 @@ module ActiveRecord
 
       # Appends a primary key definition to the table definition.
       # Can be called multiple times, but this is probably not a good idea.
-      def primary_key(name, options = {})
-        column(name, :primary_key, options)
+      def primary_key(name)
+        column(name, :primary_key)
       end
 
       # Returns a ColumnDefinition for the column with name +name+.
@@ -357,7 +357,7 @@ module ActiveRecord
       #
       # Available options are (none of these exists by default):
       # * <tt>:limit</tt> -
-      #   Requests a maximum column length. This is number of characters for <tt>:string</tt> and <tt>:text</tt> columns and number of bytes for :binary, :integer and :primary_key columns.
+      #   Requests a maximum column length. This is number of characters for <tt>:string</tt> and <tt>:text</tt> columns and number of bytes for :binary and :integer columns.
       # * <tt>:default</tt> -
       #   The column's default value. Use nil for NULL.
       # * <tt>:null</tt> -
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 12ca02924a..521bd810d0 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -183,7 +183,7 @@ module ActiveRecord
       QUOTED_TRUE, QUOTED_FALSE = '1'.freeze, '0'.freeze
 
       NATIVE_DATABASE_TYPES = {
-        :primary_key => { :name => "DEFAULT NULL auto_increment PRIMARY KEY", :limit => 4 },
+        :primary_key => "int(11) DEFAULT NULL auto_increment PRIMARY KEY".freeze,
         :string      => { :name => "varchar", :limit => 255 },
         :text        => { :name => "text" },
         :integer     => { :name => "int", :limit => 4 },
@@ -541,21 +541,15 @@ module ActiveRecord
 
       # Maps logical Rails types to MySQL-specific data types.
       def type_to_sql(type, limit = nil, precision = nil, scale = nil)
-        case type.to_s
-        when 'primary_key':
-          native = native_database_types[:primary_key]
-          return type_to_sql('integer', limit) + ' ' + native[:name]
-        when 'integer':
-          case limit
-          when 1; 'tinyint'
-          when 2; 'smallint'
-          when 3; 'mediumint'
-          when nil, 4, 11; 'int(11)'  # compatibility with MySQL default
-          when 5..8; 'bigint'
-          else raise(ActiveRecordError, "No integer type has byte size #{limit}")
-          end
-        else
-          super
+        return super unless type.to_s == 'integer'
+
+        case limit
+        when 1; 'tinyint'
+        when 2; 'smallint'
+        when 3; 'mediumint'
+        when nil, 4, 11; 'int(11)'  # compatibility with MySQL default
+        when 5..8; 'bigint'
+        else raise(ActiveRecordError, "No integer type has byte size #{limit}")
         end
       end
 
diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb
index 22cd1cbe0c..c8e1b4f53a 100644
--- a/activerecord/lib/active_record/schema_dumper.rb
+++ b/activerecord/lib/active_record/schema_dumper.rb
@@ -76,7 +76,6 @@ HEADER
       def table(table, stream)
         columns = @connection.columns(table)
         begin
-          column_specs = columns.dup
           tbl = StringIO.new
 
           # first dump primary key column
@@ -87,12 +86,9 @@ HEADER
           end
           
           tbl.print "  create_table #{table.inspect}"
-          if pkc = column_specs.detect { |c| c.name == pk }
-            if pkc.limit != @types[:primary_key][:limit]
-              tbl.print ", :id => false"
-            else
-              column_specs.delete(pkc)
-              tbl.print %Q(, :primary_key => "#{pk}") if pk != 'id'
+          if columns.detect { |c| c.name == pk }
+            if pk != 'id'
+              tbl.print %Q(, :primary_key => "#{pk}")
             end
           else
             tbl.print ", :id => false"
@@ -101,29 +97,27 @@ HEADER
           tbl.puts " do |t|"
 
           # then dump all non-primary key columns
-          column_specs.map! do |column|
+          column_specs = columns.map do |column|
             raise StandardError, "Unknown type '#{column.sql_type}' for column '#{column.name}'" if @types[column.type].nil?
+            next if column.name == pk
             spec = {}
             spec[:name]      = column.name.inspect
+            
+            # AR has an optimisation which handles zero-scale decimals as integers.  This
+            # code ensures that the dumper still dumps the column as a decimal.
+            spec[:type]      = if column.type == :integer && [/^numeric/, /^decimal/].any? { |e| e.match(column.sql_type) }
+                                 'decimal'
+                               else
+                                 column.type.to_s
+                               end
             spec[:limit]     = column.limit.inspect if column.limit != @types[column.type][:limit] && spec[:type] != 'decimal'
-            if column.name == pk
-              spec[:type]    = 'primary_key'
-            else
-              # AR has an optimisation which handles zero-scale decimals as integers.  This
-              # code ensures that the dumper still dumps the column as a decimal.
-              spec[:type]      = if column.type == :integer && [/^numeric/, /^decimal/].any? { |e| e.match(column.sql_type) }
-                                   'decimal'
-                                 else
-                                   column.type.to_s
-                                 end
-              spec[:precision] = column.precision.inspect if !column.precision.nil?
-              spec[:scale]     = column.scale.inspect if !column.scale.nil?
-              spec[:null]      = 'false' if !column.null
-              spec[:default]   = default_string(column.default) if column.has_default?
-            end
+            spec[:precision] = column.precision.inspect if !column.precision.nil?
+            spec[:scale]     = column.scale.inspect if !column.scale.nil?
+            spec[:null]      = 'false' if !column.null
+            spec[:default]   = default_string(column.default) if column.has_default?
             (spec.keys - [:name, :type]).each{ |k| spec[k].insert(0, "#{k.inspect} => ")}
             spec
-          end
+          end.compact
 
           # find all migration keys used in this table
           keys = [:name, :limit, :precision, :scale, :default, :null] & column_specs.map(&:keys).flatten
diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb
index 95039289c4..1c43e3c5b5 100644
--- a/activerecord/test/cases/schema_dumper_test.rb
+++ b/activerecord/test/cases/schema_dumper_test.rb
@@ -45,7 +45,7 @@ class SchemaDumperTest < ActiveRecord::TestCase
       next if column_set.empty?
 
       lengths = column_set.map do |column|
-        if match = column.match(/t\.(?:integer|decimal|float|datetime|timestamp|time|date|text|binary|string|boolean|primary_key)\s+"/)
+        if match = column.match(/t\.(?:integer|decimal|float|datetime|timestamp|time|date|text|binary|string|boolean)\s+"/)
           match[0].length
         end
       end
@@ -169,13 +169,6 @@ class SchemaDumperTest < ActiveRecord::TestCase
     assert_match %r(:primary_key => "movieid"), match[1], "non-standard primary key not preserved"
   end
 
-  def test_mysql_schema_dump_should_honor_primary_keys_limits
-    output = standard_dump
-    match = output.match(%r{create_table "primary_key_limit",.*?:id => false\b.*? do (.*?)\bend$}m)
-    assert_not_nil(match, output)
-    assert_match %r(t.primary_key\s+"id",\s+:limit => \d+$), match[1], "limit option not found on primary key"
-  end
-
   if current_adapter?(:MysqlAdapter)
     def test_schema_dump_should_not_add_default_value_for_mysql_text_field
       output = standard_dump
@@ -218,7 +211,7 @@ class SchemaDumperTest < ActiveRecord::TestCase
     if current_adapter?(:OracleAdapter)
       assert_match %r{t.integer\s+"atoms_in_universe",\s+:precision => 38,\s+:scale => 0}, output
     else
-      assert_match %r{t.decimal\s+"atoms_in_universe",\s+:limit => 55,\s+:precision => 55,\s+:scale => 0}, output
+      assert_match %r{t.decimal\s+"atoms_in_universe",\s+:precision => 55,\s+:scale => 0}, output
     end
   end
 
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index a1932ad16a..bec4291457 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -394,10 +394,6 @@ ActiveRecord::Schema.define do
     t.integer :price
   end
 
-  create_table :primary_key_limit, :force => true, :id => false do |t|
-    t.primary_key :id, :limit => 8
-  end
-
   create_table :projects, :force => true do |t|
     t.string :name
     t.string :type