Wrong time returned for timestamptz with :local default_timezone around DST time change

[ioev]

We're having some trouble with the timestamptz data format when using :local time in the database. I traced the issue to around here:
https://github.com/rsim/oracle-enhanced/blob/master/lib/active_record/connection_adapters/oracle_enhanced/oci_connection.rb#L261

EDIT: I updated the test case to make sure local time is set to one that uses DST, since I don't think it will fail unless this is the case.

I also included an extra spec for a case that we've also run into, where a time isn't formatted correctly for a timestamp_with_timezone column. Probably not an easy fix since I don't think the query can know at this point what the column type really is. A workaround might be to wrap the time in some other class, so that _quote can properly format it as "TO_TIMESTAMP_TZ()"?

Steps to reproduce

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails", github: "rails/rails", branch: "6-0-stable"
  gem "activerecord-oracle_enhanced-adapter",  github: "rsim/oracle-enhanced", branch: "release60"
  gem "rspec"

  platforms :ruby do
    gem "ruby-oci8"
  end
end

require "active_record"
require "rspec"
require "logger"
require "active_record/connection_adapters/oracle_enhanced_adapter"

# Set Oracle enhanced adapter specific connection parameters
DATABASE_NAME = ENV["DATABASE_NAME"] || "dev"
DATABASE_HOST = ENV["DATABASE_HOST"]
DATABASE_PORT = ENV["DATABASE_PORT"]
DATABASE_USER = ENV["DATABASE_USER"] || "oracle_enhanced"
DATABASE_PASSWORD = ENV["DATABASE_PASSWORD"] || "oracle_enhanced"
DATABASE_SYS_PASSWORD = ENV["DATABASE_SYS_PASSWORD"] || "admin"

CONNECTION_PARAMS = {
  adapter: "oracle_enhanced",
  database: DATABASE_NAME,
  host: DATABASE_HOST,
  port: DATABASE_PORT,
  username: DATABASE_USER,
  password: DATABASE_PASSWORD
}

ActiveRecord::Base.logger = Logger.new(STDOUT)
Time.zone = ActiveSupport::TimeZone.new('Central Time (US & Canada)')

describe "bug test" do
  before(:all) do
    ActiveRecord::Base.establish_connection(CONNECTION_PARAMS)
    ActiveRecord::Schema.define do
      create_table :posts, force: true do |t|
        t.timestamptz :posted_at
      end
    end

    class Post < ActiveRecord::Base
    end
  end

  it "should properly store/retrieve timestamp with tz in :local" do
    ActiveRecord::Base.default_timezone = :local
    time = Time.new(2014, 11, 2, 1, 0, 0, '-05:00')
    post = Post.create!(posted_at: time)
    post.reload
    expect(post.posted_at).to eq time
  end

  it "should be able to properly format times for timestamptz column" do
    ActiveRecord::Base.default_timezone = :utc
    time = Time.now
    post = Post.create!(posted_at: time)

    expect(Post.where('posted_at = ?', time).first).to eq post
  end
end

Expected behavior

The time should be correctly returned.

Actual behavior

Time is offset by 1 hour.

System configuration

Rails version: 6-0-stable

Oracle enhanced adapter version: 6.0.6

Ruby version: 2.7.2

Oracle Database version: 12.2.0.1.0

[yahonda]

Thanks for the report. I have attempted to reproduce it but it looks like I'm getting OCIError: ORA-01843: not a valid month not Time is offset by 1 hour..

$ more rep2147.rb
# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails", github: "rails/rails", branch: "6-0-stable"
  gem "activerecord-oracle_enhanced-adapter",  github: "rsim/oracle-enhanced", branch: "release60"
  gem "rspec"

  platforms :ruby do
    gem "ruby-oci8"
  end
end

require "active_record"
require "rspec/autorun"
require "logger"
require "active_record/connection_adapters/oracle_enhanced_adapter"

# Set Oracle enhanced adapter specific connection parameters
DATABASE_NAME = ENV["DATABASE_NAME"] || "dev"
DATABASE_HOST = ENV["DATABASE_HOST"]
DATABASE_PORT = ENV["DATABASE_PORT"]
DATABASE_USER = ENV["DATABASE_USER"] || "oracle_enhanced"
DATABASE_PASSWORD = ENV["DATABASE_PASSWORD"] || "oracle_enhanced"
DATABASE_SYS_PASSWORD = ENV["DATABASE_SYS_PASSWORD"] || "admin"

CONNECTION_PARAMS = {
  adapter: "oracle_enhanced",
  database: DATABASE_NAME,
  host: DATABASE_HOST,
  port: DATABASE_PORT,
  username: DATABASE_USER,
  password: DATABASE_PASSWORD
}

ActiveRecord::Base.logger = Logger.new(STDOUT)
Time.zone = ActiveSupport::TimeZone.new('Central Time (US & Canada)')

describe "bug test" do
  before(:all) do
    ActiveRecord::Base.establish_connection(CONNECTION_PARAMS)
    ActiveRecord::Schema.define do
      create_table :posts, force: true do |t|
        t.timestamptz :posted_at
      end
    end

    class Post < ActiveRecord::Base
    end
  end

  it "should properly store/retrieve timestamp with tz in :local" do
    ActiveRecord::Base.default_timezone = :local
    time = Time.new(2014, 11, 2, 1, 0, 0, '-05:00')
    post = Post.create!(posted_at: time)
    post.reload
    expect(post.posted_at).to eq time
  end

  it "should be able to properly format times for timestamptz column" do
    ActiveRecord::Base.default_timezone = :utc
    time = Time.now
    post = Post.create!(posted_at: time)

    expect(Post.where('posted_at = ?', time).first).to eq post
  end
end

$ ruby rep2147.rb
... snip 

Failures:

  1) bug test should be able to properly format times for timestamptz column
     Failure/Error: expect(Post.where('posted_at = ?', time).first).to eq post

     ActiveRecord::StatementInvalid:
       OCIError: ORA-01843: not a valid month
     # stmt.c:265:in oci8lib_270.so
     # rep2147.rb:70:in `block (2 levels) in <main>'
     # ------------------
     # --- Caused by: ---
     # OCIError:
     #   ORA-01843: not a valid month
     #   stmt.c:265:in oci8lib_270.so

Finished in 1.49 seconds (files took 0.24557 seconds to load)
2 examples, 1 failure

Failed examples:

rspec rep2147.rb:65 # bug test should be able to properly format times for timestamptz column

$

@ioev Would you advise what is wrong with my testcase?

[ioev]

Actually, that IS the behavior on the second test currently, because the timestamp isn't formatted correctly for the timestamptz column. I probably should have written the test a little more semantically with expect {}.not_to raise_exception.

Looks like the first test is passing for you however, so maybe something else is missing in order to configure the test to use a timezone that has DST.

[ioev]

Since we couldn't really wait, for the time being I've fixed the second problem by formatting all dates:
timestamp '%Y-%m-%d %H:%M:%S %:z'

Which seems to be accepted by the date field and the timestamp_with_tz, and doesn't seem to affect index choices as far as I can tell so far.