From 9e1740af9c1a00980b30c66aba3c223c33e646e4 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Fri, 20 Dec 2013 08:45:54 -0200 Subject: [PATCH] Tidy up fix for PG extensions quoting Always pass in the column for quote_bound_value and quote using it in case it exists there. --- activerecord/CHANGELOG.md | 4 ++-- activerecord/lib/active_record/sanitization.rb | 10 ++++++---- .../test/cases/adapters/postgresql/array_test.rb | 8 ++++++-- .../test/cases/adapters/postgresql/hstore_test.rb | 8 ++++++-- .../test/cases/adapters/postgresql/json_test.rb | 8 ++++++-- 5 files changed, 26 insertions(+), 12 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 3521e3a9ab..f08f3024b9 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,9 +1,9 @@ * Fixed `update_column`, `update_columns`, and `update_all` to correctly serialize - values for `array`, `hstore` and `json` column types in `PostgreSQL`. + values for `array`, `hstore` and `json` column types in PostgreSQL. Fixes #12261. - *Tadas Tamosauskas* + *Tadas Tamosauskas*, *Carlos Antonio da Silva* * Do not consider PostgreSQL array columns as number or text columns. diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb index bc78c3858a..dacaec26b7 100644 --- a/activerecord/lib/active_record/sanitization.rb +++ b/activerecord/lib/active_record/sanitization.rb @@ -100,8 +100,9 @@ module ActiveRecord # { status: nil, group_id: 1 } # # => "status = NULL , group_id = 1" def sanitize_sql_hash_for_assignment(attrs, table) + c = connection attrs.map do |attr, value| - "#{connection.quote_table_name_for_assignment(table, attr)} = #{quote_bound_value(value, connection, columns_hash[attr.to_s])}" + "#{c.quote_table_name_for_assignment(table, attr)} = #{quote_bound_value(value, c, columns_hash[attr.to_s])}" end.join(', ') end @@ -153,15 +154,16 @@ module ActiveRecord end def quote_bound_value(value, c = connection, column = nil) #:nodoc: - db_native_type = column && (column.respond_to?(:array) && column.array || [:json, :hstore].include?(column.type) ) - if value.respond_to?(:map) && !value.acts_like?(:string) && !db_native_type + if column + c.quote(value, column) + elsif value.respond_to?(:map) && !value.acts_like?(:string) if value.respond_to?(:empty?) && value.empty? c.quote(nil) else value.map { |v| c.quote(v) }.join(',') end else - c.quote(value, column) + c.quote(value) end end diff --git a/activerecord/test/cases/adapters/postgresql/array_test.rb b/activerecord/test/cases/adapters/postgresql/array_test.rb index cf059d20e2..d71e2aa2bb 100644 --- a/activerecord/test/cases/adapters/postgresql/array_test.rb +++ b/activerecord/test/cases/adapters/postgresql/array_test.rb @@ -129,9 +129,13 @@ class PostgresqlArrayTest < ActiveRecord::TestCase end def test_update_all - PgArray.create! tags: ["one", "two", "three"] + pg_array = PgArray.create! tags: ["one", "two", "three"] + PgArray.update_all tags: ["four", "five"] - assert_equal ["four", "five"], PgArray.first.tags + assert_equal ["four", "five"], pg_array.reload.tags + + PgArray.update_all tags: [] + assert_equal [], pg_array.reload.tags end private diff --git a/activerecord/test/cases/adapters/postgresql/hstore_test.rb b/activerecord/test/cases/adapters/postgresql/hstore_test.rb index 730020b5c8..6df5d8f533 100644 --- a/activerecord/test/cases/adapters/postgresql/hstore_test.rb +++ b/activerecord/test/cases/adapters/postgresql/hstore_test.rb @@ -208,9 +208,13 @@ class PostgresqlHstoreTest < ActiveRecord::TestCase end def test_update_all - Hstore.create! tags: { "one" => "two" } + hstore = Hstore.create! tags: { "one" => "two" } + Hstore.update_all tags: { "three" => "four" } - assert_equal({ "three" => "four" }, Hstore.first.tags) + assert_equal({ "three" => "four" }, hstore.reload.tags) + + Hstore.update_all tags: { } + assert_equal({ }, hstore.reload.tags) end end diff --git a/activerecord/test/cases/adapters/postgresql/json_test.rb b/activerecord/test/cases/adapters/postgresql/json_test.rb index 10efa8b802..01e7334aad 100644 --- a/activerecord/test/cases/adapters/postgresql/json_test.rb +++ b/activerecord/test/cases/adapters/postgresql/json_test.rb @@ -123,8 +123,12 @@ class PostgresqlJSONTest < ActiveRecord::TestCase end def test_update_all - JsonDataType.create! payload: { "one" => "two" } + json = JsonDataType.create! payload: { "one" => "two" } + JsonDataType.update_all payload: { "three" => "four" } - assert_equal({ "three" => "four" }, JsonDataType.first.payload) + assert_equal({ "three" => "four" }, json.reload.payload) + + JsonDataType.update_all payload: { } + assert_equal({ }, json.reload.payload) end end