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

Handle TIMESTAMP WITH TIMEZONE separately from TIMEZONE #1267

Merged
merged 2 commits into from
Mar 30, 2017

Conversation

yahonda
Copy link
Collaborator

@yahonda yahonda commented Mar 30, 2017

Fixes #1206

rails/rails#27962 moves TypeCastingTest from sqlite3 specific one to generic then some tests are failing because type_cast and quote method both call quoted_date for Date and Time class objects in abstract adapter but Oracle enhanced adapter does not, using its own implementation.

This pull request lets generic Date and Type objects handled by abstract adapter implementation.
For non-generic data types including timezone ActiveRecord::OracleEnhanced::Type::TimestampTz type has been created and TimestampTz type is handled as it used to.

Oracle enhanced adapter now relies on Time::DATE_FORMATS defined in ActiveSupport by this change.
Therefore changing NLS_DATE_FORMAT and NLS_TIMESTAMP_FORMAT are not working and will not be supported.

Fixes rsim#1206

rails/rails#27962 moves `TypeCastingTest` from sqlite3 specific one to generic then some tests are failing because `type_cast` and `quote` method both call `quoted_date` for `Date` and `Time` class objects in abstract adapter
but Oracle enhanced adapter does not, using its own implementation.

This pull request lets generic `Date` and `Type` objects handled by abstract adapter implementation.
For non-generic data types including timezone `ActiveRecord::OracleEnhanced::Type::TimestampTz` type has been created and `TimestampTz` type is handled as it used to.

Oracle enhanced adapter now relies on `Time::DATE_FORMATS` defined in ActiveSupport by this change.
Therefore changing `NLS_DATE_FORMAT` and `NLS_TIMESTAMP_FORMAT` are not working and will not be supported.
since Oracle enhanced adapter now relies on ActiveRecord type cast and
quote implementations.
@yahonda
Copy link
Collaborator Author

yahonda commented Mar 30, 2017

This pull request addresses these 4 failures:

 33) Failure:
FinderTest#test_hash_condition_utc_time_interpolation_with_default_timezone_local [/home/yahonda/git/rails/activerecord/test/cases/finder_test.rb:829]:
--- expected
+++ actual
@@ -1 +1 @@
-#<Topic id: 1, title: "The First Topic", author_name: "David", author_email_address: "david@loudthinking.com", written_on: "2003-07-16 14:28:11", bonus_time: "2005-01-30 14:28:00", last_read: "2004-04-15", content: "Have a nice day", important: nil, approved: false, replies_count: 1, unique_replies_count: 0, parent_id: nil, parent_title: nil, type: nil, group: nil, created_at: "2017-03-30 11:56:59", updated_at: "2017-03-30 11:56:59">
+nil


 42) Failure:
ActiveRecord::ConnectionAdapters::TypeCastingTest#test_type_cast_date [/home/yahonda/git/rails/activerecord/test/cases/quoting_test.rb:165]:
Expected: "2017-03-30"
  Actual: Thu, 30 Mar 2017


 43) Failure:
ActiveRecord::ConnectionAdapters::TypeCastingTest#test_type_cast_time [/home/yahonda/git/rails/activerecord/test/cases/quoting_test.rb:171]:
Expected: "2017-03-30 11:58:53.015137"
  Actual: 2017-03-30 11:58:53 UTC


 44) Failure:
ActiveRecord::ConnectionAdapters::QuoteARBaseTest#test_type_cast_ar_object [/home/yahonda/git/rails/activerecord/test/cases/quoting_test.rb:247]:
Expected: "2017-02-14 12:34:56.789000"
  Actual: 2017-02-14 12:34:56 UTC

@yahonda yahonda merged commit ebd7552 into rsim:master Mar 30, 2017
yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Mar 30, 2017
yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Mar 30, 2017
yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Mar 30, 2017
yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Mar 30, 2017
yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Mar 30, 2017
@yahonda yahonda deleted the type_cast_date branch March 30, 2017 20:45
yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Mar 30, 2017
cdinger added a commit to cdinger/oracle-enhanced that referenced this pull request Nov 1, 2017
According to rsim#1267, manipulating the value of `NLS_DATE_FORMAT` and
`NLS_TIMESTAMP_FORMAT` are not supported. In fact, changing
`NLS_TIMESTAMP_FORMAT` to anything but the default
(`YYYY-MM-DD HH24:MI:SS:FF6`) causes ActiveRecord to fail catastrophically.

This pulls these two unsupported variables from `DEFAULT_NLS_PARAMETERS` and
moves them the new `FIXED_NLS_PARAMETERS` to be more explicit about
this. The existings specs have been modified to use `NLS_TERRITORY`, a
supported optional parameter.

New specs have been added to ensure `NLS_DATE_FORMAT` and
`NLS_TIMESTAMP_FORMAT` never take values from environment variables or
connection parameters.
cdinger added a commit to cdinger/oracle-enhanced that referenced this pull request Nov 1, 2017
According to rsim#1267, manipulating the value of `NLS_DATE_FORMAT` and
`NLS_TIMESTAMP_FORMAT` are not supported. In fact, changing
`NLS_TIMESTAMP_FORMAT` to anything but the default
(`YYYY-MM-DD HH24:MI:SS:FF6`) causes ActiveRecord to fail catastrophically.

This pulls these two unsupported variables from `DEFAULT_NLS_PARAMETERS` and
moves them the new `FIXED_NLS_PARAMETERS` to be more explicit about
this. The existings specs have been modified to use `NLS_TERRITORY`, a
supported optional parameter.

New specs have been added to ensure `NLS_DATE_FORMAT` and
`NLS_TIMESTAMP_FORMAT` never take values from environment variables or
connection parameters.
cdinger added a commit to cdinger/oracle-enhanced that referenced this pull request Nov 1, 2017
According to rsim#1267, manipulating the value of `NLS_DATE_FORMAT` and
`NLS_TIMESTAMP_FORMAT` are not supported. In fact, changing
`NLS_TIMESTAMP_FORMAT` to anything but the default
(`YYYY-MM-DD HH24:MI:SS:FF6`) causes ActiveRecord to fail catastrophically.

This pulls these two unsupported variables from `DEFAULT_NLS_PARAMETERS` and
moves them the new `FIXED_NLS_PARAMETERS` to be more explicit about
this. The existing specs have been modified to use `NLS_TERRITORY`, a
supported optional parameter.

New specs have been added to ensure `NLS_DATE_FORMAT` and
`NLS_TIMESTAMP_FORMAT` never accept values from environment variables or
connection parameters.
yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Nov 1, 2017
cdinger added a commit to cdinger/oracle-enhanced that referenced this pull request Mar 29, 2018
According to rsim#1267, manipulating the value of `NLS_DATE_FORMAT` and
`NLS_TIMESTAMP_FORMAT` are not supported. In fact, changing
`NLS_TIMESTAMP_FORMAT` to anything but the default
(`YYYY-MM-DD HH24:MI:SS:FF6`) causes ActiveRecord to fail catastrophically.

This pulls these two unsupported variables from `DEFAULT_NLS_PARAMETERS` and
moves them the new `FIXED_NLS_PARAMETERS` to be more explicit about
this. The existing specs have been modified to use `NLS_TERRITORY`, a
supported optional parameter.

New specs have been added to ensure `NLS_DATE_FORMAT` and
`NLS_TIMESTAMP_FORMAT` never accept values from environment variables or
connection parameters.
cdinger added a commit to cdinger/oracle-enhanced that referenced this pull request Mar 30, 2018
According to rsim#1267, manipulating the value of `NLS_DATE_FORMAT` and
`NLS_TIMESTAMP_FORMAT` are not supported. In fact, changing
`NLS_TIMESTAMP_FORMAT` to anything but the default
(`YYYY-MM-DD HH24:MI:SS:FF6`) causes ActiveRecord to fail catastrophically.

This pulls these two unsupported variables from `DEFAULT_NLS_PARAMETERS` and
moves them the new `FIXED_NLS_PARAMETERS` to be more explicit about
this. The existing specs have been modified to use `NLS_TERRITORY`, a
supported optional parameter.

New specs have been added to ensure `NLS_DATE_FORMAT` and
`NLS_TIMESTAMP_FORMAT` never accept values from environment variables or
connection parameters.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant