Skip to content

Remove :timestamp column type #15184

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

Merged
merged 1 commit into from
May 19, 2014

Conversation

sgrif
Copy link
Contributor

@sgrif sgrif commented May 19, 2014

The :timestamp type for columns is unused. All database adapters treat
them as the same database type. All code in ActiveRecord which changes
its behavior based on the column's type acts the same in both cases.
However, when the type is passed to code that checks for the :datetime
type, but not :timestamp (such as XML serialization), the result is
unexpected behavior.

Existing schema definitions will continue to work, and the timestamp
type is transparently aliased to datetime.

The `:timestamp` type for columns is unused. All database adapters treat
them as the same database type. All code in `ActiveRecord` which changes
its behavior based on the column's type acts the same in both cases.
However, when the type is passed to code that checks for the `:datetime`
type, but not `:timestamp` (such as XML serialization), the result is
unexpected behavior.

Existing schema definitions will continue to work, and the `timestamp`
type is transparently aliased to `datetime`.
rafaelfranca added a commit that referenced this pull request May 19, 2014
@rafaelfranca rafaelfranca merged commit 03035d6 into rails:master May 19, 2014
@sgrif sgrif deleted the sg-remove-timestamp-type branch May 19, 2014 22:15
@yahonda
Copy link
Member

yahonda commented May 20, 2014

Oracle enhanced adapter handles timestamp as timestamp. https://github.com/rsim/oracle-enhanced/blob/master/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb#L452-L455

Since we are behind to support rails master branch, it needs to implement 4bd5dff first and let me check...

@sgrif
Copy link
Contributor Author

sgrif commented May 20, 2014

You should be able to override the aliased_types method in the oracle adapter, and just return an empty hash. https://github.com/rails/rails/pull/15184/files#diff-21d4fbe002689dc4b0ab29f021585457R297

@yahonda
Copy link
Member

yahonda commented May 20, 2014

Thanks for the advice. Let me try.

senny added a commit that referenced this pull request Nov 7, 2014
ghiculescu added a commit to ghiculescu/rails that referenced this pull request May 6, 2021
…one`

In rails#21126 it was suggested to make "timestamp with time zone" the default type for datetime columns in PostgreSQL. This is in line with PostgreSQL [best practices](https://wiki.postgresql.org/wiki/Don't_Do_This#Don.27t_use_timestamp_.28without_time_zone.29). This PR lays some groundwork for that.

This PR adds a configuration option, `ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.datetime_type`. The default is `:timestamp` which preserves current Rails behavior of using "timestamp without time zone" when you do `t.datetime` in a migration. If you change it to `:timestamptz`, you'll get "timestamp with time zone" columns instead.

If you change this setting in an existing app, you should immediately call `bin/rails db:migrate` to ensure your `schema.rb` file remains correct. If you do so, then existing columns will not be impacted, so for example if you have an app with a mixture of both types of columns, and you change the config, schema dumps will continue to output the correct types.

This PR also adds two new types that can be used in migrations: `t.timestamp` and `t.timestamptz`.

```ruby
ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.datetime_type = :timestamp # default value is :timestamp

create_table("foo1") do |t|
  t.datetime :default_format # "timestamp without time zone"
  t.timestamp :without_time_zone # "timestamp without time zone"
  t.timestamptz :with_time_zone # "timestamp with time zone"
end

ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.datetime_type = :timestamptz

create_table("foo2") do |t|
  t.datetime :default_format # "timestamp with time zone" <-- note how this has changed!
  t.timestamp :without_time_zone # "timestamp without time zone"
  t.timestamptz :with_time_zone # "timestamp with time zone"
end

ActiveRecord::ConnectionAdapters::PostgreSQLAdapter::NATIVE_DATABASE_TYPES[:my_custom_type] = { name: "custom_datetime_format_i_invented" }
ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.datetime_type = :my_custom_type

create_table("foo3") do |t|
  t.datetime :default_format # "custom_datetime_format_i_invented"
  t.timestamp :without_time_zone # "timestamp without time zone"
  t.timestamptz :with_time_zone # "timestamp with time zone"
end
```

**Notes**

- This PR doesn't change the default `datetime` format. The default is still "timestamp without time zone". A future PR could do that, but there was enough code here just getting the config option right.
- See also rails#41395 which set some groundwork (and added some tests) for this.
- This reverts some of rails#15184. rails#15184 alluded to issues in XML serialization, but I couldn't find any related tests that this broke.
stefannibrasil pushed a commit to hexdevs/rails that referenced this pull request May 27, 2021
…one`

In rails#21126 it was suggested to make "timestamp with time zone" the default type for datetime columns in PostgreSQL. This is in line with PostgreSQL [best practices](https://wiki.postgresql.org/wiki/Don't_Do_This#Don.27t_use_timestamp_.28without_time_zone.29). This PR lays some groundwork for that.

This PR adds a configuration option, `ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.datetime_type`. The default is `:timestamp` which preserves current Rails behavior of using "timestamp without time zone" when you do `t.datetime` in a migration. If you change it to `:timestamptz`, you'll get "timestamp with time zone" columns instead.

If you change this setting in an existing app, you should immediately call `bin/rails db:migrate` to ensure your `schema.rb` file remains correct. If you do so, then existing columns will not be impacted, so for example if you have an app with a mixture of both types of columns, and you change the config, schema dumps will continue to output the correct types.

This PR also adds two new types that can be used in migrations: `t.timestamp` and `t.timestamptz`.

```ruby
ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.datetime_type = :timestamp # default value is :timestamp

create_table("foo1") do |t|
  t.datetime :default_format # "timestamp without time zone"
  t.timestamp :without_time_zone # "timestamp without time zone"
  t.timestamptz :with_time_zone # "timestamp with time zone"
end

ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.datetime_type = :timestamptz

create_table("foo2") do |t|
  t.datetime :default_format # "timestamp with time zone" <-- note how this has changed!
  t.timestamp :without_time_zone # "timestamp without time zone"
  t.timestamptz :with_time_zone # "timestamp with time zone"
end

ActiveRecord::ConnectionAdapters::PostgreSQLAdapter::NATIVE_DATABASE_TYPES[:my_custom_type] = { name: "custom_datetime_format_i_invented" }
ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.datetime_type = :my_custom_type

create_table("foo3") do |t|
  t.datetime :default_format # "custom_datetime_format_i_invented"
  t.timestamp :without_time_zone # "timestamp without time zone"
  t.timestamptz :with_time_zone # "timestamp with time zone"
end
```

**Notes**

- This PR doesn't change the default `datetime` format. The default is still "timestamp without time zone". A future PR could do that, but there was enough code here just getting the config option right.
- See also rails#41395 which set some groundwork (and added some tests) for this.
- This reverts some of rails#15184. rails#15184 alluded to issues in XML serialization, but I couldn't find any related tests that this broke.
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.

3 participants