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 55d33600703f..29d85d82cbde 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 @@ def quoted_date(value) 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 932aaf7aa7b9..4f6a1ec67b69 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 @@ def prepare_column_options(column) 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 9392bcb473a6..ee11c0efa40a 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 @@ def index_algorithms {} 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 @@ def column_name_for_operation(operation, node) # :nodoc: 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 @@ def register_class_with_limit(mapping, key, klass) # :nodoc: 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 c29692d6cabb..054c59ef5c5b 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 @@ def initialize_type_map(m) # :nodoc: 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 @@ def register_integer_type(mapping, key, options) # :nodoc: 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 @@ def create_table_definition(name, temporary = false, options = nil, as = nil) # 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 @@ def cast_value(value) 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 2fe61eeb7720..b9e7894e5c98 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 OID # :nodoc: 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 e8614b16e044..a25f2521bb96 100644 --- a/activerecord/lib/active_record/type/date_time.rb +++ b/activerecord/lib/active_record/type/date_time.rb @@ -11,28 +11,25 @@ def type 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 d261e2db558b..ff8ce9324845 100644 --- a/activerecord/test/cases/adapters/mysql2/connection_test.rb +++ b/activerecord/test/cases/adapters/mysql2/connection_test.rb @@ -122,11 +122,4 @@ def test_logs_name_rename_column_sql 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 ae00f4e131f7..7a37247d6a3f 100644 --- a/activerecord/test/cases/adapters/mysql2/datetime_test.rb +++ b/activerecord/test/cases/adapters/mysql2/datetime_test.rb @@ -4,15 +4,12 @@ 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 894cf1ffa2d3..60baacf67a37 100644 --- a/activerecord/test/cases/adapters/postgresql/quoting_test.rb +++ b/activerecord/test/cases/adapters/postgresql/quoting_test.rb @@ -27,11 +27,6 @@ def test_quote_float_infinity 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 8246b14b9333..9e631fb4ca23 100644 --- a/activerecord/test/cases/adapters/postgresql/timestamp_test.rb +++ b/activerecord/test/cases/adapters/postgresql/timestamp_test.rb @@ -145,10 +145,13 @@ def test_formatting_timestamp_according_to_precision 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 ad0934051812..6d91f96bf665 100644 --- a/activerecord/test/cases/quoting_test.rb +++ b/activerecord/test/cases/quoting_test.rb @@ -46,28 +46,28 @@ def test_quoted_date 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 @@ def test_quoted_datetime_utc # 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