From b404613c977a5cc31c6748723e903fa5a0709c3b Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Sat, 28 Jun 2014 14:44:49 -0600 Subject: [PATCH] Always pass a column with a type object to quote The only case where we got a column that was not `nil`, but did not respond to `cast_type` was when type casting the default value during schema creation. We can look up the cast type, and add that object to the column definition. Will allow us to consistently rely on the type objects for type casting in all directions. --- .../connection_adapters/abstract/quoting.rb | 14 ++------------ .../abstract/schema_creation.rb | 5 +++++ .../abstract/schema_definitions.rb | 2 +- .../connection_adapters/abstract_adapter.rb | 4 ++-- .../postgresql/schema_statements.rb | 8 ++++++++ .../connection_adapters/postgresql_adapter.rb | 5 +++++ .../test/cases/adapters/sqlite3/quoting_test.rb | 4 ++-- activerecord/test/cases/quoting_test.rb | 11 ----------- 8 files changed, 25 insertions(+), 28 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb index ff92375820..bee99e5fc9 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb @@ -9,12 +9,7 @@ module ActiveRecord # records are quoted as their primary key return value.quoted_id if value.respond_to?(:quoted_id) - # FIXME: The only case we get an object other than nil or a real column - # is `SchemaStatements#add_column` with a PG array that has a non-empty default - # value. Is this really the only case? Are we missing tests for other types? - # We should have a real column object passed (or nil) here, and check for that - # instead - if column.respond_to?(:cast_type) + if column value = column.cast_type.type_cast_for_database(value) end @@ -29,12 +24,7 @@ module ActiveRecord return value.id end - # FIXME: The only case we get an object other than nil or a real column - # is `SchemaStatements#add_column` with a PG array that has a non-empty default - # value. Is this really the only case? Are we missing tests for other types? - # We should have a real column object passed (or nil) here, and check for that - # instead - if column.respond_to?(:cast_type) + if column value = column.cast_type.type_cast_for_database(value) end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb index c1379f6bec..adad6cd542 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb @@ -94,6 +94,7 @@ module ActiveRecord def quote_value(value, column) column.sql_type ||= type_to_sql(column.type, column.limit, column.precision, column.scale) + column.cast_type ||= type_for_column(column) @conn.quote(value, column) end @@ -114,6 +115,10 @@ module ActiveRecord MSG end end + + def type_for_column(column) + @conn.lookup_cast_type(column.sql_type) + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 33b522f391..98e6795f10 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -15,7 +15,7 @@ module ActiveRecord # are typically created by methods in TableDefinition, and added to the # +columns+ attribute of said TableDefinition object, in order to be used # for generating a number of table creation or table changing SQL statements. - class ColumnDefinition < Struct.new(:name, :type, :limit, :precision, :scale, :default, :null, :first, :after, :primary_key, :sql_type) #:nodoc: + class ColumnDefinition < Struct.new(:name, :type, :limit, :precision, :scale, :default, :null, :first, :after, :primary_key, :sql_type, :cast_type) #:nodoc: def primary_key? primary_key || type.to_sym == :primary_key diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 294ed6d7bf..f8c054eb69 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -375,12 +375,12 @@ module ActiveRecord Column.new(name, default, cast_type, sql_type, null) end - protected - def lookup_cast_type(sql_type) # :nodoc: type_map.lookup(sql_type) end + protected + def initialize_type_map(m) # :nodoc: register_class_with_limit m, %r(boolean)i, Type::Boolean register_class_with_limit m, %r(char)i, Type::String diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 596ebca5d6..e09672d239 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -31,6 +31,14 @@ module ActiveRecord super end end + + def type_for_column(column) + if column.array + @conn.lookup_cast_type("#{column.sql_type}[]") + else + super + end + end end module SchemaStatements diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 34262cf91d..a164758640 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -383,6 +383,11 @@ module ActiveRecord PostgreSQL::Table.new(table_name, base) end + def lookup_cast_type(sql_type) # :nodoc: + oid = execute("SELECT #{quote(sql_type)}::regtype::oid", "SCHEMA").first['oid'].to_i + super(oid) + end + protected # Returns the version of the connected PostgreSQL server. diff --git a/activerecord/test/cases/adapters/sqlite3/quoting_test.rb b/activerecord/test/cases/adapters/sqlite3/quoting_test.rb index 8c9a051eea..3bd53aa278 100644 --- a/activerecord/test/cases/adapters/sqlite3/quoting_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/quoting_test.rb @@ -15,10 +15,10 @@ module ActiveRecord def test_type_cast_binary_encoding_without_logger @conn.extend(Module.new { def logger; end }) - cast_type = Type::String.new + column = Column.new(nil, nil, Type::String.new) binary = SecureRandom.hex expected = binary.dup.encode!(Encoding::UTF_8) - assert_equal expected, @conn.type_cast(binary, cast_type) + assert_equal expected, @conn.type_cast(binary, column) end def test_type_cast_symbol diff --git a/activerecord/test/cases/quoting_test.rb b/activerecord/test/cases/quoting_test.rb index 70d9b9dbf5..1d6ae2f67f 100644 --- a/activerecord/test/cases/quoting_test.rb +++ b/activerecord/test/cases/quoting_test.rb @@ -83,12 +83,10 @@ module ActiveRecord def test_quote_with_quoted_id assert_equal 1, @quoter.quote(Struct.new(:quoted_id).new(1), nil) - assert_equal 1, @quoter.quote(Struct.new(:quoted_id).new(1), 'foo') end def test_quote_nil assert_equal 'NULL', @quoter.quote(nil, nil) - assert_equal 'NULL', @quoter.quote(nil, 'foo') end def test_quote_true @@ -102,48 +100,39 @@ module ActiveRecord def test_quote_float float = 1.2 assert_equal float.to_s, @quoter.quote(float, nil) - assert_equal float.to_s, @quoter.quote(float, Object.new) end def test_quote_fixnum fixnum = 1 assert_equal fixnum.to_s, @quoter.quote(fixnum, nil) - assert_equal fixnum.to_s, @quoter.quote(fixnum, Object.new) end def test_quote_bignum bignum = 1 << 100 assert_equal bignum.to_s, @quoter.quote(bignum, nil) - assert_equal bignum.to_s, @quoter.quote(bignum, Object.new) end def test_quote_bigdecimal bigdec = BigDecimal.new((1 << 100).to_s) assert_equal bigdec.to_s('F'), @quoter.quote(bigdec, nil) - assert_equal bigdec.to_s('F'), @quoter.quote(bigdec, Object.new) end def test_dates_and_times @quoter.extend(Module.new { def quoted_date(value) 'lol' end }) assert_equal "'lol'", @quoter.quote(Date.today, nil) - assert_equal "'lol'", @quoter.quote(Date.today, Object.new) assert_equal "'lol'", @quoter.quote(Time.now, nil) - assert_equal "'lol'", @quoter.quote(Time.now, Object.new) assert_equal "'lol'", @quoter.quote(DateTime.now, nil) - assert_equal "'lol'", @quoter.quote(DateTime.now, Object.new) end def test_crazy_object crazy = Class.new.new expected = "'#{YAML.dump(crazy)}'" assert_equal expected, @quoter.quote(crazy, nil) - assert_equal expected, @quoter.quote(crazy, Object.new) end def test_crazy_object_calls_quote_string crazy = Class.new { def initialize; @lol = 'lo\l' end }.new assert_match "lo\\\\l", @quoter.quote(crazy, nil) - assert_match "lo\\\\l", @quoter.quote(crazy, Object.new) end def test_quote_string_no_column