From f1a0fa9e19b7e4ccaea191fc6cf0613880222ee7 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Tue, 10 Feb 2015 11:52:01 -0700 Subject: [PATCH] Refactor microsecond precision to be database agnostic The various databases don't actually need significantly different handling for this behavior, and they can achieve it without knowing about the type of the object. The old implementation was returning a string, which will cause problems such as breaking TZ aware attributes, and making it impossible for the adapters to supply their logic for time objects. --- .../connection_adapters/abstract/quoting.rb | 7 +++- .../abstract/schema_dumper.rb | 2 +- .../connection_adapters/abstract_adapter.rb | 37 ++++++++----------- .../abstract_mysql_adapter.rb | 23 +++++------- .../postgresql/oid/date_time.rb | 9 ----- .../lib/active_record/type/date_time.rb | 25 ++++++------- .../cases/adapters/mysql2/connection_test.rb | 7 ---- .../cases/adapters/mysql2/datetime_test.rb | 11 ++---- .../cases/adapters/postgresql/quoting_test.rb | 5 --- .../adapters/postgresql/timestamp_test.rb | 3 ++ activerecord/test/cases/quoting_test.rb | 10 ++--- 11 files changed, 55 insertions(+), 84 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb index 55d3360070..29d85d82cb 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb @@ -127,7 +127,12 @@ module ActiveRecord end end - value.to_s(:db) + result = value.to_s(:db) + if value.respond_to?(:usec) && value.usec > 0 + "#{result}.#{sprintf("%06d", value.usec)}" + else + result + end end def prepare_binds_for_database(binds) # :nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb index 932aaf7aa7..4f6a1ec67b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb @@ -29,7 +29,7 @@ module ActiveRecord limit = column.limit || native_database_types[column.type][:limit] spec[:limit] = limit.inspect if limit - spec[:precision] = column.precision.inspect if column.precision + spec[:precision] = column.precision.inspect if column.precision && column.precision != 0 spec[:scale] = column.scale.inspect if column.scale default = schema_default(column) if column.has_default? diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 9392bcb473..ee11c0efa4 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -263,18 +263,6 @@ module ActiveRecord {} end - # QUOTING ================================================== - - # Quote date/time values for use in SQL input. Includes microseconds - # if the value is a Time responding to usec. - def quoted_date(value) #:nodoc: - if value.acts_like?(:time) && value.respond_to?(:usec) - "#{super}.#{sprintf("%06d", value.usec)}" - else - super - end - end - # Returns a bind substitution value given a bind +column+ # NOTE: The column param is currently being used by the sqlserver-adapter def substitute_at(column, _unused = 0) @@ -409,15 +397,15 @@ module ActiveRecord 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 - register_class_with_limit m, %r(binary)i, Type::Binary - register_class_with_limit m, %r(text)i, Type::Text - register_class_with_limit m, %r(date)i, Type::Date - register_class_with_limit m, %r(time)i, Type::Time - register_class_with_limit m, %r(datetime)i, Type::DateTime - register_class_with_limit m, %r(float)i, Type::Float - register_class_with_limit m, %r(int)i, Type::Integer + register_class_with_limit m, %r(boolean)i, Type::Boolean + register_class_with_limit m, %r(char)i, Type::String + register_class_with_limit m, %r(binary)i, Type::Binary + register_class_with_limit m, %r(text)i, Type::Text + register_class_with_precision m, %r(date)i, Type::Date + register_class_with_precision m, %r(time)i, Type::Time + register_class_with_precision m, %r(datetime)i, Type::DateTime + register_class_with_limit m, %r(float)i, Type::Float + register_class_with_limit m, %r(int)i, Type::Integer m.alias_type %r(blob)i, 'binary' m.alias_type %r(clob)i, 'text' @@ -451,6 +439,13 @@ module ActiveRecord end end + def register_class_with_precision(mapping, key, klass) # :nodoc: + mapping.register_type(key) do |*args| + precision = extract_precision(args.last) + klass.new(precision: precision) + end + end + def extract_scale(sql_type) # :nodoc: case sql_type when /\((\d+)\)/ then 0 diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index c29692d6ca..054c59ef5c 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -712,11 +712,6 @@ module ActiveRecord m.alias_type %r(year)i, 'integer' m.alias_type %r(bit)i, 'binary' - m.register_type(%r(datetime)i) do |sql_type| - precision = extract_precision(sql_type) - MysqlDateTime.new(precision: precision) - end - m.register_type(%r(enum)i) do |sql_type| limit = sql_type[/^enum\((.+)\)/i, 1] .split(',').map{|enum| enum.strip.length - 2}.max @@ -734,6 +729,14 @@ module ActiveRecord end end + def extract_precision(sql_type) + if /datetime/ === sql_type + super || 0 + else + super + end + end + def fetch_type_metadata(sql_type, collation = "", extra = "") MysqlTypeMetadata.new(super(sql_type), collation: collation, extra: extra, strict: strict_mode?) end @@ -916,14 +919,6 @@ module ActiveRecord TableDefinition.new(native_database_types, name, temporary, options, as) end - class MysqlDateTime < Type::DateTime # :nodoc: - private - - def has_precision? - precision || 0 - end - end - class MysqlString < Type::String # :nodoc: def type_cast_for_database(value) case value @@ -945,7 +940,7 @@ module ActiveRecord end def type_classes_with_standard_constructor - super.merge(string: MysqlString, date_time: MysqlDateTime) + super.merge(string: MysqlString) end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/date_time.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/date_time.rb index 2fe61eeb77..b9e7894e5c 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/date_time.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/date_time.rb @@ -5,15 +5,6 @@ module ActiveRecord class DateTime < Type::DateTime # :nodoc: include Infinity - def type_cast_for_database(value) - if has_precision? && value.acts_like?(:time) && value.year <= 0 - bce_year = format("%04d", -value.year + 1) - super.sub(/^-?\d+/, bce_year) + " BC" - else - super - end - end - def cast_value(value) if value.is_a?(::String) case value diff --git a/activerecord/lib/active_record/type/date_time.rb b/activerecord/lib/active_record/type/date_time.rb index e8614b16e0..a25f2521bb 100644 --- a/activerecord/lib/active_record/type/date_time.rb +++ b/activerecord/lib/active_record/type/date_time.rb @@ -11,28 +11,25 @@ module ActiveRecord end def type_cast_for_database(value) - return super unless value.acts_like?(:time) - - zone_conversion_method = ActiveRecord::Base.default_timezone == :utc ? :getutc : :getlocal - - if value.respond_to?(zone_conversion_method) - value = value.send(zone_conversion_method) + if precision && value.respond_to?(:usec) + number_of_insignificant_digits = 6 - precision + round_power = 10 ** number_of_insignificant_digits + value = value.change(usec: value.usec / round_power * round_power) end - return value unless has_precision? + if value.acts_like?(:time) + zone_conversion_method = ActiveRecord::Base.default_timezone == :utc ? :getutc : :getlocal - result = value.to_s(:db) - if value.respond_to?(:usec) && (1..6).cover?(precision) - "#{result}.#{sprintf("%0#{precision}d", value.usec / 10 ** (6 - precision))}" - else - result + if value.respond_to?(zone_conversion_method) + value = value.send(zone_conversion_method) + end end + + value end private - alias has_precision? precision - def cast_value(string) return string unless string.is_a?(::String) return if string.empty? diff --git a/activerecord/test/cases/adapters/mysql2/connection_test.rb b/activerecord/test/cases/adapters/mysql2/connection_test.rb index d261e2db55..ff8ce93248 100644 --- a/activerecord/test/cases/adapters/mysql2/connection_test.rb +++ b/activerecord/test/cases/adapters/mysql2/connection_test.rb @@ -122,11 +122,4 @@ class MysqlConnectionTest < ActiveRecord::TestCase ensure @connection.execute "DROP TABLE `bar_baz`" end - - if mysql_56? - def test_quote_time_usec - assert_equal "'1970-01-01 00:00:00.000000'", @connection.quote(Time.at(0)) - assert_equal "'1970-01-01 00:00:00.000000'", @connection.quote(Time.at(0).to_datetime) - end - end end diff --git a/activerecord/test/cases/adapters/mysql2/datetime_test.rb b/activerecord/test/cases/adapters/mysql2/datetime_test.rb index ae00f4e131..7a37247d6a 100644 --- a/activerecord/test/cases/adapters/mysql2/datetime_test.rb +++ b/activerecord/test/cases/adapters/mysql2/datetime_test.rb @@ -4,15 +4,12 @@ if mysql_56? class DateTimeTest < ActiveRecord::TestCase self.use_transactional_fixtures = false - class Foo < ActiveRecord::Base; end - - def test_default_datetime_precision - ActiveRecord::Base.connection.create_table(:foos, force: true) - ActiveRecord::Base.connection.add_column :foos, :created_at, :datetime - ActiveRecord::Base.connection.add_column :foos, :updated_at, :datetime - assert_nil activerecord_column_option('foos', 'created_at', 'precision') + teardown do + ActiveRecord::Base.connection.drop_table(:foos, if_exists: true) end + class Foo < ActiveRecord::Base; end + def test_datetime_data_type_with_precision ActiveRecord::Base.connection.create_table(:foos, force: true) ActiveRecord::Base.connection.add_column :foos, :created_at, :datetime, precision: 1 diff --git a/activerecord/test/cases/adapters/postgresql/quoting_test.rb b/activerecord/test/cases/adapters/postgresql/quoting_test.rb index 894cf1ffa2..60baacf67a 100644 --- a/activerecord/test/cases/adapters/postgresql/quoting_test.rb +++ b/activerecord/test/cases/adapters/postgresql/quoting_test.rb @@ -27,11 +27,6 @@ module ActiveRecord assert_equal "'Infinity'", @conn.quote(infinity) end - def test_quote_time_usec - assert_equal "'1970-01-01 00:00:00.000000'", @conn.quote(Time.at(0)) - assert_equal "'1970-01-01 00:00:00.000000'", @conn.quote(Time.at(0).to_datetime) - end - def test_quote_range range = "1,2]'; SELECT * FROM users; --".."a" type = OID::Range.new(Type::Integer.new, :int8range) diff --git a/activerecord/test/cases/adapters/postgresql/timestamp_test.rb b/activerecord/test/cases/adapters/postgresql/timestamp_test.rb index 8246b14b93..9e631fb4ca 100644 --- a/activerecord/test/cases/adapters/postgresql/timestamp_test.rb +++ b/activerecord/test/cases/adapters/postgresql/timestamp_test.rb @@ -145,10 +145,13 @@ class TimestampTest < ActiveRecord::TestCase date = ::Time.utc(2014, 8, 17, 12, 30, 0, 999999) Foo.create!(created_at: date, updated_at: date) assert foo = Foo.find_by(created_at: date) + assert_equal 1, Foo.where(updated_at: date).count assert_equal date.to_s, foo.created_at.to_s assert_equal date.to_s, foo.updated_at.to_s assert_equal 000000, foo.created_at.usec assert_equal 999900, foo.updated_at.usec + ensure + ActiveRecord::Base.connection.drop_table(:foos, if_exists: true) end private diff --git a/activerecord/test/cases/quoting_test.rb b/activerecord/test/cases/quoting_test.rb index ad09340518..6d91f96bf6 100644 --- a/activerecord/test/cases/quoting_test.rb +++ b/activerecord/test/cases/quoting_test.rb @@ -46,28 +46,28 @@ module ActiveRecord def test_quoted_time_utc with_timezone_config default: :utc do - t = Time.now + t = Time.now.change(usec: 0) assert_equal t.getutc.to_s(:db), @quoter.quoted_date(t) end end def test_quoted_time_local with_timezone_config default: :local do - t = Time.now + t = Time.now.change(usec: 0) assert_equal t.getlocal.to_s(:db), @quoter.quoted_date(t) end end def test_quoted_time_crazy with_timezone_config default: :asdfasdf do - t = Time.now + t = Time.now.change(usec: 0) assert_equal t.getlocal.to_s(:db), @quoter.quoted_date(t) end end def test_quoted_datetime_utc with_timezone_config default: :utc do - t = DateTime.now + t = Time.now.change(usec: 0).to_datetime assert_equal t.getutc.to_s(:db), @quoter.quoted_date(t) end end @@ -76,7 +76,7 @@ module ActiveRecord # DateTime doesn't define getlocal, so make sure it does nothing def test_quoted_datetime_local with_timezone_config default: :local do - t = DateTime.now + t = Time.now.change(usec: 0).to_datetime assert_equal t.to_s(:db), @quoter.quoted_date(t) end end