Skip to content

Commit

Permalink
Default timestamps to non-null
Browse files Browse the repository at this point in the history
  • Loading branch information
mperham committed Oct 15, 2011
1 parent e759c88 commit 3dbedd2
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ def #{column_type}(*args) # def st
# Appends <tt>:datetime</tt> columns <tt>:created_at</tt> and
# <tt>:updated_at</tt> to the table.
def timestamps(*args)
options = args.extract_options!
options = { :null => false }.merge(args.extract_options!)
column(:created_at, :datetime, options)
column(:updated_at, :datetime, options)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -507,8 +507,8 @@ def distinct(columns, order_by)
# ===== Examples
# add_timestamps(:suppliers)
def add_timestamps(table_name)
add_column table_name, :created_at, :datetime
add_column table_name, :updated_at, :datetime
add_column table_name, :created_at, :datetime, :null => false
add_column table_name, :updated_at, :datetime, :null => false
end

# Removes the timestamp columns (created_at and updated_at) from the table definition.
Expand Down
10 changes: 5 additions & 5 deletions activerecord/test/cases/migration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -389,8 +389,8 @@ def test_create_table_with_timestamps_should_create_datetime_columns
created_at_column = created_columns.detect {|c| c.name == 'created_at' }
updated_at_column = created_columns.detect {|c| c.name == 'updated_at' }

assert created_at_column.null
assert updated_at_column.null
assert !created_at_column.null
assert !updated_at_column.null
ensure
Person.connection.drop_table table_name rescue nil
end
Expand Down Expand Up @@ -471,11 +471,11 @@ def test_native_decimal_insert_manual_vs_automatic

# Do a manual insertion
if current_adapter?(:OracleAdapter)
Person.connection.execute "insert into people (id, wealth) values (people_seq.nextval, 12345678901234567890.0123456789)"
Person.connection.execute "insert into people (id, wealth, created_at, updated_at) values (people_seq.nextval, 12345678901234567890.0123456789, 0, 0)"
elsif current_adapter?(:OpenBaseAdapter) || (current_adapter?(:MysqlAdapter) && Mysql.client_version < 50003) #before mysql 5.0.3 decimals stored as strings
Person.connection.execute "insert into people (wealth) values ('12345678901234567890.0123456789')"
Person.connection.execute "insert into people (wealth, created_at, updated_at) values ('12345678901234567890.0123456789', 0, 0)"
else
Person.connection.execute "insert into people (wealth) values (12345678901234567890.0123456789)"
Person.connection.execute "insert into people (wealth, created_at, updated_at) values (12345678901234567890.0123456789, 0, 0)"
end

# SELECT
Expand Down

4 comments on commit 3dbedd2

@vjebelev
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get a fresh test failure under postgres (9.1) that I suspect is caused by this commit:

vlad@mbp:activerecord> rake postgresql:rebuild_databases
vlad@mbp:activerecord> ARCONN=postgresql ruby -Itest test/cases/migration_test.rb
Using postgresql with Identity Map off
Loaded suite test/cases/migration_test
Started
.....................................................................................................E.............................
Finished in 18.715265 seconds.

  1. Error:
    test_native_decimal_insert_manual_vs_automatic(MigrationTest):
    ActiveRecord::StatementInvalid: PGError: ERROR: column "created_at" is of type timestamp without time zone but expression is of type integer
    LINE 1: ..., updated_at) values (12345678901234567890.0123456789, 0, 0)
    ^
    HINT: You will need to rewrite or cast the expression.
    : insert into people (wealth, created_at, updated_at) values (12345678901234567890.0123456789, 0, 0)
    /web/rails/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb:581:in async_exec' /web/rails/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb:581:inblock in execute'
    /web/rails/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:257:in block in log' /web/rails/activesupport/lib/active_support/notifications/instrumenter.rb:20:ininstrument'
    /web/rails/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:252:in log' /web/rails/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb:580:inexecute'
    test/cases/migration_test.rb:478:in test_native_decimal_insert_manual_vs_automatic' /Users/vlad/.rvm/gems/ruby-1.9.2-p180/gems/mocha-0.10.0/lib/mocha/integration/mini_test/version_142_to_172.rb:27:inrun'
    /web/rails/activesupport/lib/active_support/testing/setup_and_teardown.rb:35:in block in run' /web/rails/activesupport/lib/active_support/callbacks.rb:408:in_run_setup_callbacks'
    /web/rails/activesupport/lib/active_support/callbacks.rb:81:in run_callbacks' /web/rails/activesupport/lib/active_support/testing/setup_and_teardown.rb:34:inrun'

131 tests, 372 assertions, 0 failures, 1 errors, 0 skips

Test run options: --seed 35916

@yahonda
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've got a same error with PostgreSQL 9.1 and Oracle. Then opened a pull request #3349.

@pcreux
Copy link
Contributor

@pcreux pcreux commented on 3dbedd2 Jan 23, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mperham What was the reason for this change? This seems like a sane default to me.

@pcreux
Copy link
Contributor

@pcreux pcreux commented on 3dbedd2 Jan 23, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mperham Sorry - this comment was supposed to go on the REVERT of this commit. 😕

Please sign in to comment.