From c462c2bd07548449131359b8e507ce11a44e6cb5 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Tue, 17 Jun 2014 16:37:08 -0600 Subject: [PATCH] Ensure `OID::Array#type_cast_for_database` matches PG's quoting behavior Also takes a step towards supporting types which use a character other than ',' for the delimiter (`box` is the only built in type for which this is the case) --- .../postgresql/oid/array.rb | 26 ++++++++++++++----- .../postgresql/oid/type_map_initializer.rb | 2 +- .../cases/adapters/postgresql/array_test.rb | 18 +++++++++++++ .../adapters/postgresql/type_lookup_test.rb | 15 +++++++++++ 4 files changed, 54 insertions(+), 7 deletions(-) create mode 100644 activerecord/test/cases/adapters/postgresql/type_lookup_test.rb 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