diff options
Diffstat (limited to 'activerecord')
6 files changed, 76 insertions, 6 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 6d130ab4d6..f08f3024b9 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,28 @@ +* Fixed `update_column`, `update_columns`, and `update_all` to correctly serialize + values for `array`, `hstore` and `json` column types in PostgreSQL. + + Fixes #12261. + + *Tadas Tamosauskas*, *Carlos Antonio da Silva* + +* Do not consider PostgreSQL array columns as number or text columns. + + The code uses these checks in several places to know what to do with a + particular column, for instance AR attribute query methods has a branch + like this: + + if column.number? + !value.zero? + end + + This should never be true for array columns, since it would be the same + as running [].zero?, which results in a NoMethodError exception. + + Fixing this by ensuring that array columns in PostgreSQL never return + true for number?/text? checks. + + *Carlos Antonio da Silva* + * When connecting to a non-existant postgresql database, the error: `ActiveRecord::NoDatabaseError` will now be raised. When being used with Rails the error message will include information on how to create a database: diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 95cd69d5ad..11a5eba464 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -46,7 +46,7 @@ module ActiveRecord # PostgreSQL-specific extensions to column definitions in a table. class PostgreSQLColumn < Column #:nodoc: attr_accessor :array - # Instantiates a new PostgreSQL column definition in a table. + def initialize(name, default, oid_type, sql_type = nil, null = true) @oid_type = oid_type default_value = self.class.extract_value_from_default(default) @@ -62,6 +62,14 @@ module ActiveRecord @default_function = default if has_default_function?(default_value, default) end + def number? + !array && super + end + + def text? + !array && super + end + # :stopdoc: class << self include ConnectionAdapters::PostgreSQLColumn::Cast diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb index cab8fd745a..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)}" + "#{c.quote_table_name_for_assignment(table, attr)} = #{quote_bound_value(value, c, columns_hash[attr.to_s])}" end.join(', ') end @@ -152,8 +153,10 @@ module ActiveRecord end end - def quote_bound_value(value, c = connection) #:nodoc: - if value.respond_to?(:map) && !value.acts_like?(:string) + def quote_bound_value(value, c = connection, column = nil) #:nodoc: + 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 diff --git a/activerecord/test/cases/adapters/postgresql/array_test.rb b/activerecord/test/cases/adapters/postgresql/array_test.rb index 114d5b6cc6..d71e2aa2bb 100644 --- a/activerecord/test/cases/adapters/postgresql/array_test.rb +++ b/activerecord/test/cases/adapters/postgresql/array_test.rb @@ -26,6 +26,12 @@ class PostgresqlArrayTest < ActiveRecord::TestCase def test_column assert_equal :string, @column.type assert @column.array + assert_not @column.text? + + ratings_column = PgArray.columns_hash['ratings'] + assert_equal :integer, ratings_column.type + assert ratings_column.array + assert_not ratings_column.number? end def test_change_column_with_array @@ -50,8 +56,6 @@ class PostgresqlArrayTest < ActiveRecord::TestCase end def test_type_cast_array - assert @column - data = '{1,2,3}' oid_type = @column.instance_variable_get('@oid_type').subtype # we are getting the instance variable in this test, but in the @@ -124,6 +128,16 @@ class PostgresqlArrayTest < ActiveRecord::TestCase assert_equal("[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ...]", record.attribute_for_inspect(:ratings)) end + def test_update_all + pg_array = PgArray.create! tags: ["one", "two", "three"] + + PgArray.update_all tags: ["four", "five"] + assert_equal ["four", "five"], pg_array.reload.tags + + PgArray.update_all tags: [] + assert_equal [], pg_array.reload.tags + end + private def assert_cycle field, array # test creation diff --git a/activerecord/test/cases/adapters/postgresql/hstore_test.rb b/activerecord/test/cases/adapters/postgresql/hstore_test.rb index 2845413575..6df5d8f533 100644 --- a/activerecord/test/cases/adapters/postgresql/hstore_test.rb +++ b/activerecord/test/cases/adapters/postgresql/hstore_test.rb @@ -206,6 +206,16 @@ class PostgresqlHstoreTest < ActiveRecord::TestCase def test_multiline assert_cycle("a\nb" => "c\nd") end + + def test_update_all + hstore = Hstore.create! tags: { "one" => "two" } + + Hstore.update_all tags: { "three" => "four" } + assert_equal({ "three" => "four" }, hstore.reload.tags) + + Hstore.update_all tags: { } + assert_equal({ }, hstore.reload.tags) + end end private diff --git a/activerecord/test/cases/adapters/postgresql/json_test.rb b/activerecord/test/cases/adapters/postgresql/json_test.rb index c33c7ef968..01e7334aad 100644 --- a/activerecord/test/cases/adapters/postgresql/json_test.rb +++ b/activerecord/test/cases/adapters/postgresql/json_test.rb @@ -121,4 +121,14 @@ class PostgresqlJSONTest < ActiveRecord::TestCase x = JsonDataType.first assert_equal "640×1136", x.resolution end + + def test_update_all + json = JsonDataType.create! payload: { "one" => "two" } + + JsonDataType.update_all payload: { "three" => "four" } + assert_equal({ "three" => "four" }, json.reload.payload) + + JsonDataType.update_all payload: { } + assert_equal({ }, json.reload.payload) + end end |