Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PostgreSQL: Ensure the database time zone matches Ruby's time zone #28391

Closed

Conversation

kamipo
Copy link
Member

@kamipo kamipo commented Mar 12, 2017

This restores bf6661c (reverts 7a7c608).

If AR::Base.default_timezone = :local, incorrect value will be saved
unless the database time zone matches Ruby's time zone.

@pixeltrix
Copy link
Contributor

Since TZ can be set via ENV how does that affect things? Obviously this affects the related MySQL PR too.

@kamipo
Copy link
Member Author

kamipo commented Mar 12, 2017

Loading development environment (Rails 5.1.0.beta1)
irb(main):001:0> conn = ActiveRecord::Base.connection
=> #<ActiveRecord::ConnectionAdapters::SQLite3Adapter:0x007fc602edbe60 @transaction_manager=#<ActiveRecord::ConnectionAdapters::TransactionManager:0x007fc602ec10d8 @stack=[], @connection=#<ActiveRecord::ConnectionAdapters::SQLite3Adapter:0x007fc602edbe60 ...>>, @query_cache={}, @query_cache_enabled=false, @connection=#<SQLite3::Database:0x007fc602ee0118 @tracefunc=nil, @authorizer=nil, @encoding=#<Encoding:UTF-8>, @busy_handler=nil, @collations={}, @functions={}, @results_as_hash=true, @type_translation=nil, @readonly=false>, @owner=#<Thread:0x007fc60287f3a0 run>, @instrumenter=#<ActiveSupport::Notifications::Instrumenter:0x007fc60706a728 @id="9d283cffef99e245098c", @notifier=#<ActiveSupport::Notifications::Fanout:0x007fc602a78558 @subscribers=[#<ActiveSupport::Notifications::Fanout::Subscribers::Evented:0x007fc606088828 @pattern="sql.active_record", @delegate=#<ActiveRecord::LogSubscriber:0x007fc606088b70 @queue_key="ActiveRecord::LogSubscriber-70244240737720", @patterns=["sql.active_record"]>, @can_publish=false>, #<ActiveSupport::Notifications::Fanout::Subscribers::Evented:0x007fc603583560 @pattern="logger.action_view", @delegate=#<ActionView::LogSubscriber:0x007fc6035839e8 @root=nil, @queue_key="ActionView::LogSubscriber-70244218182900", @patterns=["logger.action_view", "render_collection.action_view", "render_partial.action_view", "render_template.action_view"]>, @can_publish=false>, #<ActiveSupport::Notifications::Fanout::Subscribers::Evented:0x007fc603583178 @pattern="render_collection.action_view", @delegate=#<ActionView::LogSubscriber:0x007fc6035839e8 @root=nil, @queue_key="ActionView::LogSubscriber-70244218182900", @patterns=["logger.action_view", "render_collection.action_view", "render_partial.action_view", "render_template.action_view"]>, @can_publish=false>, #<ActiveSupport::Notifications::Fanout::Subscribers::Evented:0x007fc603582de0 @pattern="render_partial.action_view", @delegate=#<ActionView::LogSubscriber:0x007fc6035839e8 @root=nil, @queue_key="ActionView::LogSubscriber-70244218182900", @patterns=["logger.action_view", "render_collection.action_view", "render_partial.action_view", "render_template.action_view"]>, @can_publish=false>, #<ActiveSupport::Notifications::Fanout::Subscribers::Evented:0x007fc603582958 @pattern="render_template.action_view", @delegate=#<ActionView::LogSubscriber:0x007fc6035839e8 @root=nil, @queue_key="ActionView::LogSubscriber-70244218182900", @patterns=["logger.action_view", "render_collection.action_view", "render_partial.action_view", "render_template.action_view"]>, @can_publish=false>, #<ActiveSupport::Notifications::Fanout::Subscribers::Evented:0x007fc606404178 @pattern="sql.active_record", @delegate=#<ActiveRecord::ExplainSubscriber:0x007fc6064041f0>, @can_publish=false>], @listeners_for=#<Concurrent::Map:0x007fc602a78468 entries=2 default_proc=nil>, @_mutex=#<Thread::Mutex:0x007fc602a78058>>>, @logger=#<ActiveSupport::Logger:0x007fc602b6a470 @progname=nil, @level=0, @default_formatter=#<Logger::Formatter:0x007fc602b6a3f8 @datetime_format=nil>, @formatter=#<ActiveSupport::Logger::SimpleFormatter:0x007fc60350a728 @datetime_format=nil, @thread_key="activesupport_tagged_logging_tags:70244217934740">, @logdev=#<Logger::LogDevice:0x007fc602b6a3a8 @shift_size=nil, @shift_age=nil, @filename=nil, @dev=#<File:/Users/kamipo/work/issue_25174/log/development.log>, @mon_owner=nil, @mon_count=0, @mon_mutex=#<Thread::Mutex:0x007fc602b6a358>>, @local_levels=#<Concurrent::Map:0x007fc602b6a2b8 entries=0 default_proc=nil>>, @config={:adapter=>"sqlite3", :pool=>5, :timeout=>5000, :database=>"/Users/kamipo/work/issue_25174/db/development.sqlite3"}, @pool=#<ActiveRecord::ConnectionAdapters::ConnectionPool:0x007fc60706a430 @mon_owner=nil, @mon_count=0, @mon_mutex=#<Thread::Mutex:0x007fc60706a318>, @query_cache_enabled=#<Concurrent::Map:0x007fc60706a2f0 entries=0 default_proc=#<Proc:0x007fc60706a2a0@/Users/kamipo/work/issue_25174/vendor/bundle/ruby/2.3.0/gems/activerecord-5.1.0.beta1/lib/active_record/connection_adapters/abstract/query_cache.rb:27>>, @spec=#<ActiveRecord::ConnectionAdapters::ConnectionSpecification:0x007fc60706a8e0 @name="primary", @config={:adapter=>"sqlite3", :pool=>5, :timeout=>5000, :database=>"/Users/kamipo/work/issue_25174/db/development.sqlite3"}, @adapter_method="sqlite3_connection">, @checkout_timeout=5, @reaper=#<ActiveRecord::ConnectionAdapters::ConnectionPool::Reaper:0x007fc60706a228 @pool=#<ActiveRecord::ConnectionAdapters::ConnectionPool:0x007fc60706a430 ...>, @frequency=nil>, @size=5, @thread_cached_conns=#<Concurrent::Map:0x007fc60706a1d8 entries=1 default_proc=nil>, @connections=[#<ActiveRecord::ConnectionAdapters::SQLite3Adapter:0x007fc602edbe60 ...>], @automatic_reconnect=true, @now_connecting=0, @threads_blocking_new_connections=0, @available=#<ActiveRecord::ConnectionAdapters::ConnectionPool::ConnectionLeasingQueue:0x007fc60706a138 @lock=#<ActiveRecord::ConnectionAdapters::ConnectionPool:0x007fc60706a430 ...>, @cond=#<MonitorMixin::ConditionVariable:0x007fc60706a0e8 @monitor=#<ActiveRecord::ConnectionAdapters::ConnectionPool:0x007fc60706a430 ...>, @cond=#<Thread::ConditionVariable:0x007fc60706a0c0>>, @num_waiting=0, @queue=[]>, @lock_thread=false>, @schema_cache=#<ActiveRecord::ConnectionAdapters::SchemaCache:0x007fc602ec0f98 @connection=#<ActiveRecord::ConnectionAdapters::SQLite3Adapter:0x007fc602edbe60 ...>, @columns={}, @columns_hash={}, @primary_keys={}, @data_sources={}>, @quoted_table_names={}, @quoted_column_names={}, @visitor=#<Arel::Visitors::SQLite:0x007fc602ec0e80 @dispatch={}, @connection=#<ActiveRecord::ConnectionAdapters::SQLite3Adapter:0x007fc602edbe60 ...>>, @lock=#<Monitor:0x007fc602ec0de0 @mon_owner=nil, @mon_count=0, @mon_mutex=#<Thread::Mutex:0x007fc602ec0d40>>, @prepared_statements=true, @active=nil, @statements=#<ActiveRecord::ConnectionAdapters::SQLite3Adapter::StatementPool:0x007fc602ec0ca0 @cache={}, @statement_limit=1000>>
irb(main):002:0> type = ActiveRecord::Type.lookup(:datetime)
=> #<ActiveRecord::Type::DateTime:0x007fc604045fe8 @precision=nil, @scale=nil, @limit=nil>
irb(main):003:0> time = Time.current
=> Sun, 12 Mar 2017 18:10:08 UTC +00:00
irb(main):004:0> ActiveRecord::Base.default_timezone = :utc
=> :utc
irb(main):005:0> ENV['TZ'] = "America/New_York"
=> "America/New_York"
irb(main):006:0> conn.type_cast(type.serialize(time))
=> "2017-03-12 18:10:08.666901"
irb(main):007:0> ActiveRecord::Base.default_timezone = :local
=> :local
irb(main):008:0> conn.type_cast(type.serialize(time))
=> "2017-03-12 14:10:08.666901"

If AR::Base.default_timezone = :utc, conn.type_cast(type.serialize(time)) generates "2017-03-12 18:10:08.666901". Then database regard the time as UTC time.

But if AR::Base.default_timezone = :local, conn.type_cast(type.serialize(time)) generates "2017-03-12 14:10:08.666901" as a local time (-04:00). If the database time zone is Japan (+09:00), it will deviate 13 hours.

The database time zone should match Ruby's time zone to avoid the issue.

@kamipo kamipo force-pushed the ensure_database_tz_matches_ruby_tz branch from a35cb92 to 7fb7f6c Compare March 14, 2017 19:19
@kamipo kamipo force-pushed the ensure_database_tz_matches_ruby_tz branch from 7fb7f6c to aa4cd56 Compare March 14, 2017 20:03
@kamipo kamipo force-pushed the ensure_database_tz_matches_ruby_tz branch from aa4cd56 to ea70fda Compare March 26, 2017 14:27
kamipo added a commit to kamipo/rails that referenced this pull request Mar 26, 2017
Follow up of rails#23553. Related rails#28391.

If `AR::Base.default_timezone = :local`, incorrect value will be saved
unless the database time zone matches Ruby's time zone.
@matthewd
Copy link
Member

matthewd commented Apr 3, 2017

Couple of doubts:

  1. I'm concerned this could change the interpretation of people's existing data, if they have a working setup.

  2. It ignores DST (and any other offset changes that may occur after boot.. but DST is obviously the most likely to occur).

  3. From memory, I thought the connection timezone was mostly relevant for timestamp without timezone. timestamptz refers to an unambiguous moment in time, regardless of TZ... if we're not round-tripping through that, it sounds more like a casting issue than a configuration one. 😕

This restores bf6661c (reverts 7a7c608).

If `AR::Base.default_timezone = :local`, incorrect value will be saved
unless the database time zone matches Ruby's time zone.
@kamipo kamipo force-pushed the ensure_database_tz_matches_ruby_tz branch from ea70fda to f19295b Compare August 21, 2017 10:00
@rails-bot
Copy link

rails-bot bot commented Dec 18, 2019

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@rails-bot rails-bot bot added the stale label Dec 18, 2019
@rails-bot rails-bot bot closed this Dec 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants