From f63545ded0499c5e62f5783e76688bd7020dc220 Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Fri, 26 Nov 2021 05:04:13 +1030 Subject: [PATCH 1/3] Allow default_timezone to vary between databases Now that multiple databases are more commonly supported, it seems less reasonable to assume they all agree on stored timezone. Ultimately we might need to further accomodate differing definitions of "local", but this is a start. --- .../connection_adapters/abstract/quoting.rb | 2 +- .../connection_adapters/abstract_adapter.rb | 16 ++++++++++++++++ .../mysql/database_statements.rb | 4 ++-- .../connection_adapters/mysql/quoting.rb | 2 +- .../connection_adapters/postgresql_adapter.rb | 10 +++++----- activerecord/lib/active_record/integration.rb | 2 +- activerecord/lib/active_record/timestamp.rb | 2 +- activerecord/test/cases/quoting_test.rb | 8 +++++++- 8 files changed, 34 insertions(+), 12 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb index 928a72181c..a211587f70 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb @@ -121,7 +121,7 @@ module ActiveRecord # if the value is a Time responding to usec. def quoted_date(value) if value.acts_like?(:time) - if ActiveRecord.default_timezone == :utc + if default_timezone == :utc value = value.getutc if !value.utc? else value = value.getlocal diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 9239e9eea3..544362c97c 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -62,6 +62,16 @@ module ActiveRecord end end + def self.validate_default_timezone(config) + case config + when nil + when "utc", "local" + config.to_sym + else + raise ArgumentError, "default_timezone must be either 'utc' or 'local'" + end + end + DEFAULT_READ_QUERY = [:begin, :commit, :explain, :release, :rollback, :savepoint, :select, :with] # :nodoc: private_constant :DEFAULT_READ_QUERY @@ -100,6 +110,8 @@ module ActiveRecord @advisory_locks_enabled = self.class.type_cast_config_to_boolean( config.fetch(:advisory_locks, true) ) + + @default_timezone = self.class.validate_default_timezone(config[:default_timezone]) end EXCEPTION_NEVER = { Exception => :never }.freeze # :nodoc: @@ -129,6 +141,10 @@ module ActiveRecord @config.fetch(:use_metadata_table, true) end + def default_timezone + @default_timezone || ActiveRecord.default_timezone + end + # Determines whether writes are currently being prevented. # # Returns true if the connection is a replica. diff --git a/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb index a6cd13077f..f4db966e5d 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb @@ -91,7 +91,7 @@ module ActiveRecord def raw_execute(sql, name, async: false) # make sure we carry over any changes to ActiveRecord.default_timezone that have been # made since we established the connection - @connection.query_options[:database_timezone] = ActiveRecord.default_timezone + @connection.query_options[:database_timezone] = default_timezone super end @@ -172,7 +172,7 @@ module ActiveRecord # make sure we carry over any changes to ActiveRecord.default_timezone that have been # made since we established the connection - @connection.query_options[:database_timezone] = ActiveRecord.default_timezone + @connection.query_options[:database_timezone] = default_timezone type_casted_binds = type_casted_binds(binds) diff --git a/activerecord/lib/active_record/connection_adapters/mysql/quoting.rb b/activerecord/lib/active_record/connection_adapters/mysql/quoting.rb index f93cbf7e47..3316b5e3d0 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/quoting.rb @@ -57,7 +57,7 @@ module ActiveRecord # We need to check explicitly for ActiveSupport::TimeWithZone because # we need to transform it to Time objects but we don't want to # transform Time objects to themselves. - if ActiveRecord.default_timezone == :utc + if default_timezone == :utc value.getutc else value.getlocal diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index c7a1838985..39473eddf4 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -872,7 +872,7 @@ module ActiveRecord # If using Active Record's time zone support configure the connection to return # TIMESTAMP WITH ZONE types in UTC. unless variables["timezone"] - if ActiveRecord.default_timezone == :utc + if default_timezone == :utc variables["timezone"] = "UTC" elsif @local_tz variables["timezone"] = @local_tz @@ -972,15 +972,15 @@ module ActiveRecord end def update_typemap_for_default_timezone - if @default_timezone != ActiveRecord.default_timezone && @timestamp_decoder - decoder_class = ActiveRecord.default_timezone == :utc ? + if @mapped_default_timezone != default_timezone && @timestamp_decoder + decoder_class = default_timezone == :utc ? PG::TextDecoder::TimestampUtc : PG::TextDecoder::TimestampWithoutTimeZone @timestamp_decoder = decoder_class.new(@timestamp_decoder.to_h) @connection.type_map_for_results.add_coder(@timestamp_decoder) - @default_timezone = ActiveRecord.default_timezone + @mapped_default_timezone = default_timezone # if default timezone has changed, we need to reconfigure the connection # (specifically, the session time zone) @@ -989,7 +989,7 @@ module ActiveRecord end def add_pg_decoders - @default_timezone = nil + @mapped_default_timezone = nil @timestamp_decoder = nil coders_by_name = { diff --git a/activerecord/lib/active_record/integration.rb b/activerecord/lib/active_record/integration.rb index b82b38d402..61b6976f7c 100644 --- a/activerecord/lib/active_record/integration.rb +++ b/activerecord/lib/active_record/integration.rb @@ -178,7 +178,7 @@ module ActiveRecord def can_use_fast_cache_version?(timestamp) timestamp.is_a?(String) && cache_timestamp_format == :usec && - ActiveRecord.default_timezone == :utc && + self.class.connection.default_timezone == :utc && !updated_at_came_from_user? end diff --git a/activerecord/lib/active_record/timestamp.rb b/activerecord/lib/active_record/timestamp.rb index a8e4f76bac..64ab5af8c2 100644 --- a/activerecord/lib/active_record/timestamp.rb +++ b/activerecord/lib/active_record/timestamp.rb @@ -75,7 +75,7 @@ module ActiveRecord end def current_time_from_proper_timezone - ActiveRecord.default_timezone == :utc ? Time.now.utc : Time.now + connection.default_timezone == :utc ? Time.now.utc : Time.now end private diff --git a/activerecord/test/cases/quoting_test.rb b/activerecord/test/cases/quoting_test.rb index cea079bb6b..1a49d73a18 100644 --- a/activerecord/test/cases/quoting_test.rb +++ b/activerecord/test/cases/quoting_test.rb @@ -6,7 +6,13 @@ module ActiveRecord module ConnectionAdapters class QuotingTest < ActiveRecord::TestCase def setup - @quoter = Class.new { include Quoting }.new + @quoter = Class.new { + include Quoting + + def default_timezone + ActiveRecord.default_timezone + end + }.new end def test_quoted_true From 8c99b749b856ac3071fb0decf4f2f806a16f08f1 Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Fri, 10 Dec 2021 21:30:43 +1030 Subject: [PATCH 2/3] Respect connection's configured timezone in its typemap --- .../connection_adapters/abstract_adapter.rb | 37 ++++++++++++---- .../abstract_mysql_adapter.rb | 21 +++++++--- .../connection_adapters/postgresql_adapter.rb | 9 ++-- .../connection_adapters/sqlite3_adapter.rb | 5 +-- .../active_record/type/internal/timezone.rb | 9 +++- activerecord/test/cases/base_test.rb | 42 +++++++++++++++++++ 6 files changed, 100 insertions(+), 23 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 544362c97c..e162d776df 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -687,6 +687,21 @@ module ActiveRecord end class << self + def register_class_with_precision(mapping, key, klass, **kwargs) # :nodoc: + mapping.register_type(key) do |*args| + precision = extract_precision(args.last) + klass.new(precision: precision, **kwargs) + end + end + + def extended_type_map(default_timezone:) # :nodoc: + Type::TypeMap.new(self::TYPE_MAP).tap do |m| + register_class_with_precision m, %r(time)i, Type::Time, timezone: default_timezone + register_class_with_precision m, %r(datetime)i, Type::DateTime, timezone: default_timezone + m.alias_type %r(timestamp)i, "datetime" + end + end + private def initialize_type_map(m) register_class_with_limit m, %r(boolean)i, Type::Boolean @@ -728,13 +743,6 @@ module ActiveRecord end end - def register_class_with_precision(mapping, key, klass) - mapping.register_type(key) do |*args| - precision = extract_precision(args.last) - klass.new(precision: precision) - end - end - def extract_scale(sql_type) case sql_type when /\((\d+)\)/ then 0 @@ -752,10 +760,23 @@ module ActiveRecord end TYPE_MAP = Type::TypeMap.new.tap { |m| initialize_type_map(m) } + EXTENDED_TYPE_MAPS = Concurrent::Map.new private + def extended_type_map_key + if @default_timezone + { default_timezone: @default_timezone } + end + end + def type_map - TYPE_MAP + if key = extended_type_map_key + self.class::EXTENDED_TYPE_MAPS.compute_if_absent(key) do + self.class.extended_type_map(**key) + end + else + self.class::TYPE_MAP + end end def translate_exception_class(e, sql, binds) 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 2dc9fb2513..187c252b02 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -561,6 +561,14 @@ module ActiveRecord end class << self + def extended_type_map(default_timezone: nil, emulate_booleans:) # :nodoc: + super(default_timezone: default_timezone).tap do |m| + if emulate_booleans + m.register_type %r(^tinyint\(1\))i, Type::Boolean.new + end + end + end + private def initialize_type_map(m) super @@ -614,13 +622,16 @@ module ActiveRecord end TYPE_MAP = Type::TypeMap.new.tap { |m| initialize_type_map(m) } - TYPE_MAP_WITH_BOOLEAN = Type::TypeMap.new(TYPE_MAP).tap do |m| - m.register_type %r(^tinyint\(1\))i, Type::Boolean.new - end + EXTENDED_TYPE_MAPS = Concurrent::Map.new + EMULATE_BOOLEANS_TRUE = { emulate_booleans: true }.freeze private - def type_map - emulate_booleans ? TYPE_MAP_WITH_BOOLEAN : TYPE_MAP + def extended_type_map_key + if @default_timezone + { default_timezone: @default_timezone, emulate_booleans: emulate_booleans } + elsif emulate_booleans + EMULATE_BOOLEANS_TRUE + end end def raw_execute(sql, name, async: false) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 39473eddf4..480e8e951f 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -575,10 +575,6 @@ module ActiveRecord m.register_type "polygon", OID::SpecializedString.new(:polygon) m.register_type "circle", OID::SpecializedString.new(:circle) - register_class_with_precision m, "time", Type::Time - register_class_with_precision m, "timestamp", OID::Timestamp - register_class_with_precision m, "timestamptz", OID::TimestampWithTimeZone - m.register_type "numeric" do |_, fmod, sql_type| precision = extract_precision(sql_type) scale = extract_scale(sql_type) @@ -613,6 +609,11 @@ module ActiveRecord def initialize_type_map(m = type_map) self.class.initialize_type_map(m) + + self.class.register_class_with_precision m, "time", Type::Time, timezone: @default_timezone + self.class.register_class_with_precision m, "timestamp", OID::Timestamp, timezone: @default_timezone + self.class.register_class_with_precision m, "timestamptz", OID::TimestampWithTimeZone + load_additional_types end diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index ef8fcece99..37ee26eaae 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -370,12 +370,9 @@ module ActiveRecord end TYPE_MAP = Type::TypeMap.new.tap { |m| initialize_type_map(m) } + EXTENDED_TYPE_MAPS = Concurrent::Map.new private - def type_map - TYPE_MAP - end - # See https://www.sqlite.org/limits.html, # the default value is 999 when not configured. def bind_params_length diff --git a/activerecord/lib/active_record/type/internal/timezone.rb b/activerecord/lib/active_record/type/internal/timezone.rb index 43ae9a0b09..98a4c5b61b 100644 --- a/activerecord/lib/active_record/type/internal/timezone.rb +++ b/activerecord/lib/active_record/type/internal/timezone.rb @@ -4,12 +4,17 @@ module ActiveRecord module Type module Internal module Timezone + def initialize(timezone: nil, **kwargs) + super(**kwargs) + @timezone = timezone + end + def is_utc? - ActiveRecord.default_timezone == :utc + default_timezone == :utc end def default_timezone - ActiveRecord.default_timezone + @timezone || ActiveRecord.default_timezone end end end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 8a03664ba5..c389b5f27d 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -982,6 +982,48 @@ class BasicsTest < ActiveRecord::TestCase end end end + + unless in_memory_db? + def test_connection_in_local_time + with_timezone_config default: :utc do + new_config = ActiveRecord::Base.connection_db_config.configuration_hash.merge(default_timezone: "local") + ActiveRecord::Base.establish_connection(new_config) + Default.reset_column_information + + default = Default.new + + assert_equal Date.new(2004, 1, 1), default.fixed_date + assert_equal Time.local(2004, 1, 1, 0, 0, 0, 0), default.fixed_time + + if current_adapter?(:PostgreSQLAdapter) + assert_equal Time.utc(2004, 1, 1, 0, 0, 0, 0), default.fixed_time_with_time_zone + end + end + ensure + ActiveRecord::Base.establish_connection :arunit + Default.reset_column_information + end + + def test_connection_in_utc_time + with_timezone_config default: :local do + new_config = ActiveRecord::Base.connection_db_config.configuration_hash.merge(default_timezone: "utc") + ActiveRecord::Base.establish_connection(new_config) + Default.reset_column_information + + default = Default.new + + assert_equal Date.new(2004, 1, 1), default.fixed_date + assert_equal Time.utc(2004, 1, 1, 0, 0, 0, 0), default.fixed_time + + if current_adapter?(:PostgreSQLAdapter) + assert_equal Time.utc(2004, 1, 1, 0, 0, 0, 0), default.fixed_time_with_time_zone + end + end + ensure + ActiveRecord::Base.establish_connection :arunit + Default.reset_column_information + end + end end def test_auto_id From efe62900c82a696172f75ae5f3fd0873ad317f47 Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Fri, 10 Dec 2021 23:18:01 +1030 Subject: [PATCH 3/3] Don't match "time" inside SET()/ENUM() We previously relied upon definition order to prevent this, but now we're defining these types in a child TypeMap, so that won't work. The loose patterns we define here seem like they'd be in general danger of this sort of issue, but for now we can just address the immediately affected ones. --- .../active_record/connection_adapters/abstract_adapter.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index e162d776df..c55e12121f 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -696,9 +696,9 @@ module ActiveRecord def extended_type_map(default_timezone:) # :nodoc: Type::TypeMap.new(self::TYPE_MAP).tap do |m| - register_class_with_precision m, %r(time)i, Type::Time, timezone: default_timezone - register_class_with_precision m, %r(datetime)i, Type::DateTime, timezone: default_timezone - m.alias_type %r(timestamp)i, "datetime" + register_class_with_precision m, %r(\A[^\(]*time)i, Type::Time, timezone: default_timezone + register_class_with_precision m, %r(\A[^\(]*datetime)i, Type::DateTime, timezone: default_timezone + m.alias_type %r(\A[^\(]*timestamp)i, "datetime" end end