Merge pull request #15784 from sgrif/sg-delimiter

Ensure `OID::Array#type_cast_for_database` matches PG's quoting behavior

4 files changed, 54 insertions, 7 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/array.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/array.rb
index b686e1e5ad..63f3be45de 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/array.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/array.rb
@@ -3,11 +3,12 @@ module ActiveRecord
     module PostgreSQL
       module OID # :nodoc:
         class Array < Type::Value
-          attr_reader :subtype
+          attr_reader :subtype, :delimiter
           delegate :type, to: :subtype
 
-          def initialize(subtype)
+          def initialize(subtype, delimiter = ',')
             @subtype = subtype
+            @delimiter = delimiter
           end
 
           def type_cast_from_database(value)
@@ -55,7 +56,7 @@ module ActiveRecord
           def cast_value_for_database(value)
             if value.is_a?(::Array)
               casted_values = value.map { |item| cast_value_for_database(item) }
-              "{#{casted_values.join(',')}}"
+              "{#{casted_values.join(delimiter)}}"
             else
               quote_and_escape(subtype.type_cast_for_database(value))
             end
@@ -66,13 +67,26 @@ module ActiveRecord
           def quote_and_escape(value)
             case value
             when ::String
-              value = value.gsub(/\\/, ARRAY_ESCAPE)
-              value.gsub!(/"/,"\\\"")
-              %("#{value}")
+              if string_requires_quoting?(value)
+                value = value.gsub(/\\/, ARRAY_ESCAPE)
+                value.gsub!(/"/,"\\\"")
+                %("#{value}")
+              else
+                value
+              end
             when nil then "NULL"
             else value
             end
           end
+
+          # See http://www.postgresql.org/docs/9.2/static/arrays.html#ARRAYS-IO
+          # for a list of all cases in which strings will be quoted.
+          def string_requires_quoting?(string)
+            string.empty? ||
+              string == "NULL" ||
+              string =~ /[\{\}"\\\s]/ ||
+              string.include?(delimiter)
+          end
         end
       end
     end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/type_map_initializer.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/type_map_initializer.rb
index 28f7a4eafb..e396ff4a1e 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/type_map_initializer.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/type_map_initializer.rb
@@ -40,7 +40,7 @@ module ActiveRecord
 
           def register_array_type(row)
             if subtype = @store.lookup(row['typelem'].to_i)
-              register row['oid'], OID::Array.new(subtype)
+              register row['oid'], OID::Array.new(subtype, row['typdelim'])
             end
           end
 
diff --git a/activerecord/test/cases/adapters/postgresql/array_test.rb b/activerecord/test/cases/adapters/postgresql/array_test.rb
index 0b1e3295cc..15901b389e 100644
--- a/activerecord/test/cases/adapters/postgresql/array_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/array_test.rb
@@ -3,6 +3,7 @@ require "cases/helper"
 
 class PostgresqlArrayTest < ActiveRecord::TestCase
   include InTimeZone
+  OID = ActiveRecord::ConnectionAdapters::PostgreSQL::OID
 
   class PgArray < ActiveRecord::Base
     self.table_name = 'pg_arrays'
@@ -203,6 +204,23 @@ class PostgresqlArrayTest < ActiveRecord::TestCase
     assert_equal tags, ar.tags
   end
 
+  def test_string_quoting_rules_match_pg_behavior
+    tags = ["", "one{", "two}", %(three"), "four\\", "five ", "six\t", "seven\n", "eight,", "nine", "ten\r", "NULL"]
+    x = PgArray.create!(tags: tags)
+    x.reload
+
+    assert_equal x.tags_before_type_cast, PgArray.columns_hash['tags'].type_cast_for_database(tags)
+  end
+
+  def test_quoting_non_standard_delimiters
+    strings = ["hello,", "world;"]
+    comma_delim = OID::Array.new(ActiveRecord::Type::String.new, ',')
+    semicolon_delim = OID::Array.new(ActiveRecord::Type::String.new, ';')
+
+    assert_equal %({"hello,",world;}), comma_delim.type_cast_for_database(strings)
+    assert_equal %({hello,;"world;"}), semicolon_delim.type_cast_for_database(strings)
+  end
+
   def test_datetime_with_timezone_awareness
     tz = "Pacific Time (US & Canada)"
 
diff --git a/activerecord/test/cases/adapters/postgresql/type_lookup_test.rb b/activerecord/test/cases/adapters/postgresql/type_lookup_test.rb
new file mode 100644
index 0000000000..23817198b1
--- /dev/null
+++ b/activerecord/test/cases/adapters/postgresql/type_lookup_test.rb
@@ -0,0 +1,15 @@
+require 'cases/helper'
+
+class PostgresqlTypeLookupTest < ActiveRecord::TestCase
+  setup do
+    @connection = ActiveRecord::Base.connection
+  end
+
+  test "array delimiters are looked up correctly" do
+    box_array = @connection.type_map.lookup(1020)
+    int_array = @connection.type_map.lookup(1007)
+
+    assert_equal ';', box_array.delimiter
+    assert_equal ',', int_array.delimiter
+  end
+end