Merge pull request #6192 from danmcclain/add_inet_and_cidr_types_to_postgresql_adapter

Add support for macaddr, inet, and cidr types to PostgreSQL adapter

5 files changed, 97 insertions, 15 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/column.rb b/activerecord/lib/active_record/connection_adapters/column.rb
index 1933ce2b46..01bd3ae26c 100644
--- a/activerecord/lib/active_record/connection_adapters/column.rb
+++ b/activerecord/lib/active_record/connection_adapters/column.rb
@@ -124,6 +124,7 @@ module ActiveRecord
         when :binary               then "#{klass}.binary_to_string(#{var_name})"
         when :boolean              then "#{klass}.value_to_boolean(#{var_name})"
         when :hstore               then "#{klass}.string_to_hstore(#{var_name})"
+        when :inet, :cidr          then "#{klass}.string_to_cidr(#{var_name})"
         else var_name
         end
       end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb
index c82afc232c..df3d5e4657 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb
@@ -137,6 +137,14 @@ module ActiveRecord
           end
         end
 
+        class Cidr < Type
+          def type_cast(value)
+            return if value.nil?
+
+            ConnectionAdapters::PostgreSQLColumn.string_to_cidr value
+          end
+        end
+
         class TypeMap
           def initialize
             @mapping = {}
@@ -212,11 +220,9 @@ module ActiveRecord
         # FIXME: why are we keeping these types as strings?
         alias_type 'tsvector', 'text'
         alias_type 'interval', 'text'
-        alias_type 'cidr',     'text'
-        alias_type 'inet',     'text'
-        alias_type 'macaddr',  'text'
         alias_type 'bit',      'text'
         alias_type 'varbit',   'text'
+        alias_type 'macaddr',  'text'
 
         # FIXME: I don't think this is correct. We should probably be returning a parsed date,
         # but the tests pass with a string returned.
@@ -237,6 +243,9 @@ module ActiveRecord
         register_type 'polygon', OID::Identity.new
         register_type 'circle', OID::Identity.new
         register_type 'hstore', OID::Hstore.new
+
+        register_type 'cidr', OID::Cidr.new
+        alias_type 'inet', 'cidr'
       end
     end
   end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 4d5459939b..6714970103 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -8,6 +8,8 @@ require 'arel/visitors/bind_visitor'
 gem 'pg', '~> 0.11'
 require 'pg'
 
+require 'ipaddr'
+
 module ActiveRecord
   module ConnectionHandling
     # Establishes a connection to the database that's used by all Active Record objects
@@ -79,6 +81,25 @@ module ActiveRecord
           end
         end
 
+        def string_to_cidr(string)
+          if string.nil?
+            nil
+          elsif String === string
+            IPAddr.new(string)
+          else
+            string
+          end
+
+        end
+
+        def cidr_to_string(object)
+          if IPAddr === object
+            "#{object.to_s}/#{object.instance_variable_get(:@mask_addr).to_s(2).count('1')}"
+          else
+            object
+          end
+        end
+
         private
         HstorePair = begin
           quoted_string = /"[^"\\]*(?:\\.[^"\\]*)*"/
@@ -197,6 +218,13 @@ module ActiveRecord
           :decimal
         when 'hstore'
           :hstore
+        # Network address types
+        when 'inet'
+          :inet
+        when 'cidr'
+          :cidr
+        when 'macaddr'
+          :macaddr
         # Character types
         when /^(?:character varying|bpchar)(?:\(\d+\))?$/
           :string
@@ -211,9 +239,6 @@ module ActiveRecord
         # Geometric types
         when /^(?:point|line|lseg|box|"?path"?|polygon|circle)$/
           :string
-        # Network address types
-        when /^(?:cidr|inet|macaddr)$/
-          :string
         # Bit strings
         when /^bit(?: varying)?(?:\(\d+\))?$/
           :string
@@ -282,6 +307,18 @@ module ActiveRecord
         def hstore(name, options = {})
           column(name, 'hstore', options)
         end
+
+        def inet(name, options = {})
+          column(name, 'inet', options)
+        end
+
+        def cidr(name, options = {})
+          column(name, 'cidr', options)
+        end
+
+        def macaddr(name, options = {})
+          column(name, 'macaddr', options)
+        end
       end
 
       ADAPTER_NAME = 'PostgreSQL'
@@ -301,7 +338,10 @@ module ActiveRecord
         :boolean     => { :name => "boolean" },
         :xml         => { :name => "xml" },
         :tsvector    => { :name => "tsvector" },
-        :hstore      => { :name => "hstore" }
+        :hstore      => { :name => "hstore" },
+        :inet        => { :name => "inet" },
+        :cidr        => { :name => "cidr" },
+        :macaddr     => { :name => "macaddr" }
       }
 
       # Returns 'PostgreSQL' as adapter name for identification purposes.
@@ -510,6 +550,11 @@ module ActiveRecord
           when 'hstore' then super(PostgreSQLColumn.hstore_to_string(value), column)
           else super
           end
+        when IPAddr
+          case column.sql_type
+          when 'inet', 'cidr' then super(PostgreSQLColumn.cidr_to_string(value), column)
+          else super
+          end
         when Float
           if value.infinite? && column.type == :datetime
             "'#{value.to_s.downcase}'"
@@ -549,6 +594,9 @@ module ActiveRecord
         when Hash
           return super unless 'hstore' == column.sql_type
           PostgreSQLColumn.hstore_to_string(value)
+        when IPAddr
+          return super unless ['inet','cidr'].includes? column.sql_type
+          PostgreSQLColumn.cidr_to_string(value)
         else
           super
         end
diff --git a/activerecord/test/cases/adapters/postgresql/datatype_test.rb b/activerecord/test/cases/adapters/postgresql/datatype_test.rb
index ce08e4c6a7..34660577da 100644
--- a/activerecord/test/cases/adapters/postgresql/datatype_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/datatype_test.rb
@@ -86,9 +86,9 @@ class PostgresqlDataTypeTest < ActiveRecord::TestCase
   end
 
   def test_data_type_of_network_address_types
-    assert_equal :string, @first_network_address.column_for_attribute(:cidr_address).type
-    assert_equal :string, @first_network_address.column_for_attribute(:inet_address).type
-    assert_equal :string, @first_network_address.column_for_attribute(:mac_address).type
+    assert_equal :cidr, @first_network_address.column_for_attribute(:cidr_address).type
+    assert_equal :inet, @first_network_address.column_for_attribute(:inet_address).type
+    assert_equal :macaddr, @first_network_address.column_for_attribute(:mac_address).type
   end
 
   def test_data_type_of_bit_string_types
@@ -134,9 +134,12 @@ class PostgresqlDataTypeTest < ActiveRecord::TestCase
     assert_equal '-1 years -2 days', @first_time.time_interval
   end
 
-  def test_network_address_values
-    assert_equal '192.168.0.0/24', @first_network_address.cidr_address
-    assert_equal '172.16.1.254', @first_network_address.inet_address
+  def test_network_address_values_ipaddr
+    cidr_address = IPAddr.new '192.168.0.0/24'
+    inet_address = IPAddr.new '172.16.1.254'
+
+    assert_equal cidr_address, @first_network_address.cidr_address
+    assert_equal inet_address, @first_network_address.inet_address
     assert_equal '01:23:45:67:89:0a', @first_network_address.mac_address
   end
 
@@ -200,8 +203,8 @@ class PostgresqlDataTypeTest < ActiveRecord::TestCase
   end
 
   def test_update_network_address
-    new_cidr_address = '10.1.2.3/32'
-    new_inet_address = '10.0.0.0/8'
+    new_inet_address = '10.1.2.3/32'
+    new_cidr_address = '10.0.0.0/8'
     new_mac_address = 'bc:de:f0:12:34:56'
     assert @first_network_address.cidr_address = new_cidr_address
     assert @first_network_address.inet_address = new_inet_address
diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb
index 15ceaa1fcc..ab80dd1d6d 100644
--- a/activerecord/test/cases/schema_dumper_test.rb
+++ b/activerecord/test/cases/schema_dumper_test.rb
@@ -236,6 +236,27 @@ class SchemaDumperTest < ActiveRecord::TestCase
       end
     end
 
+    def test_schema_dump_includes_inet_shorthand_definition
+      output = standard_dump
+      if %r{create_table "postgresql_network_address"} =~ output
+        assert_match %r{t.inet "inet_address"}, output
+      end
+    end
+
+    def test_schema_dump_includes_cidr_shorthand_definition
+      output = standard_dump
+      if %r{create_table "postgresql_network_address"} =~ output
+        assert_match %r{t.cidr "cidr_address"}, output
+      end
+    end
+
+    def test_schema_dump_includes_macaddr_shorthand_definition
+      output = standard_dump
+      if %r{create_table "postgresql_network_address"} =~ output
+        assert_match %r{t.macaddr "macaddr_address"}, output
+      end
+    end
+
     def test_schema_dump_includes_hstores_shorthand_definition
       output = standard_dump
       if %r{create_table "postgresql_hstores"} =~ output