diff options
Diffstat (limited to 'activerecord')
| -rw-r--r-- | activerecord/CHANGELOG | 6 | ||||
| -rwxr-xr-x | activerecord/lib/active_record/base.rb | 17 | ||||
| -rw-r--r-- | activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb | 57 | ||||
| -rwxr-xr-x | activerecord/test/base_test.rb | 6 | ||||
| -rwxr-xr-x | activerecord/test/fixtures_test.rb | 48 | ||||
| -rw-r--r-- | activerecord/test/migration_test.rb | 8 |
6 files changed, 103 insertions, 39 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 175e6f5bd5..70e6e8b32c 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,11 @@ *SVN* +* Base.reset_sequence_name analogous to reset_table_name (mostly useful for testing). Base.define_attr_method allows nil values. [Jeremy Kemper] + +* PostgreSQL: smarter sequence name defaults, stricter last_insert_id, warn on pk without sequence. [Jeremy Kemper] + +* PostgreSQL: correctly discover custom primary key sequences. #2594 [Blair Zajac <blair@orcaware.com>, meadow.nnick@gmail.com, Jeremy Kemper] + * SQLServer: don't report limits for unsupported field types. #2835 [Ryan Tomayko] * Include the Enumerable module in ActiveRecord::Errors. [Rick Bradley <rick@rickbradley.com>] diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 7c89ba452a..e9d885f313 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -646,9 +646,16 @@ module ActiveRecord #:nodoc: "type" end - # Default sequence_name. Use set_sequence_name to override. + # Lazy-set the sequence name to the connection's default. This method + # is only ever called once since set_sequence_name overrides it. def sequence_name - connection.default_sequence_name(table_name, primary_key) + reset_sequence_name + end + + def reset_sequence_name + default = connection.default_sequence_name(table_name, primary_key) + set_sequence_name(default) + default end # Sets the table name to use to the given value, or (if the value @@ -1053,12 +1060,12 @@ module ActiveRecord #:nodoc: def define_attr_method(name, value=nil, &block) sing = class << self; self; end sing.send :alias_method, "original_#{name}", name - if value + if block_given? + sing.send :define_method, name, &block + else # use eval instead of a block to work around a memory leak in dev # mode in fcgi sing.class_eval "def #{name}; #{value.to_s.inspect}; end" - else - sing.send :define_method, name, &block end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 8c3cc69ec9..dc5dc4032e 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -102,7 +102,7 @@ module ActiveRecord def insert(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil) #:nodoc: execute(sql, name) table = sql.split(" ", 4)[2] - id_value || last_insert_id(table, sequence_name) + id_value || last_insert_id(table, sequence_name || default_sequence_name(table, pk)) end def query(sql, name = nil) #:nodoc: @@ -199,23 +199,34 @@ module ActiveRecord @schema_search_path ||= query('SHOW search_path')[0][0] end - def default_sequence_name(table_name, pk = 'id') - "#{table_name}_#{pk}_seq" + def default_sequence_name(table_name, pk = nil) + default_pk, default_seq = pk_and_sequence_for(table_name) + default_seq || "#{table_name}_#{pk || default_pk}_seq" end - # Set the sequence to the max value of the table's pk. - def reset_pk_sequence!(table) - pk, sequence = pk_and_sequence_for(table) - if pk and sequence - select_value <<-end_sql, 'Reset sequence' - SELECT setval('#{sequence}', (SELECT COALESCE(MAX(#{pk})+(SELECT increment_by FROM #{sequence}), (SELECT min_value FROM #{sequence})) FROM #{table}), false) - end_sql + # Resets sequence to the max value of the table's pk if present. + def reset_pk_sequence!(table, pk = nil, sequence = nil) + unless pk and sequence + default_pk, default_sequence = pk_and_sequence_for(table) + pk ||= default_pk + sequence ||= default_sequence + end + if pk + if sequence + select_value <<-end_sql, 'Reset sequence' + SELECT setval('#{sequence}', (SELECT COALESCE(MAX(#{pk})+(SELECT increment_by FROM #{sequence}), (SELECT min_value FROM #{sequence})) FROM #{table}), false) + end_sql + else + @logger.warn "#{table} has primary key #{pk} with no default sequence" if @logger + end end end # Find a table's primary key and sequence. def pk_and_sequence_for(table) - execute(<<-end_sql, 'Find pk sequence')[0] + # First try looking for a sequence with a dependency on the + # given table's primary key. + result = execute(<<-end_sql, 'PK and serial sequence')[0] SELECT attr.attname, (name.nspname || '.' || seq.relname) FROM pg_class seq, pg_attribute attr, @@ -232,11 +243,29 @@ module ActiveRecord AND cons.contype = 'p' AND dep.refobjid = '#{table}'::regclass end_sql + + if result.nil? or result.empty? + # If that fails, try parsing the primary key's default value. + # Support the 7.x and 8.0 nextval('foo'::text) as well as + # the 8.1+ nextval('foo'::regclass). + # TODO: assumes sequence is in same schema as table. + result = execute(<<-end_sql, 'PK and custom sequence')[0] + SELECT attr.attname, (name.nspname || '.' || split_part(def.adsrc, '\\\'', 2)) + FROM pg_class t + JOIN pg_namespace name ON (t.relnamespace = name.oid) + JOIN pg_attribute attr ON (t.oid = attrelid) + JOIN pg_attrdef def ON (adrelid = attrelid AND adnum = attnum) + JOIN pg_constraint cons ON (conrelid = adrelid AND adnum = conkey[1]) + WHERE t.oid = '#{table}'::regclass + AND cons.contype = 'p' + AND def.adsrc ~ 'nextval\\\\(\\\'[^\\\']*\\\'::[^\\\\)]*\\\\)' + end_sql + end + result rescue nil end - def rename_table(name, new_name) execute "ALTER TABLE #{name} RENAME TO #{new_name}" end @@ -281,9 +310,7 @@ module ActiveRecord BYTEA_COLUMN_TYPE_OID = 17 def last_insert_id(table, sequence_name) - if sequence_name - @connection.exec("SELECT currval('#{sequence_name}')")[0][0].to_i - end + Integer(@connection.exec("SELECT currval('#{sequence_name}')")[0][0]) end def select(sql, name = nil) diff --git a/activerecord/test/base_test.rb b/activerecord/test/base_test.rb index f64821361f..7be6441f1d 100755 --- a/activerecord/test/base_test.rb +++ b/activerecord/test/base_test.rb @@ -839,10 +839,10 @@ class BasicsTest < Test::Unit::TestCase def test_column_name_properly_quoted col_record = ColumnName.new col_record.references = 40 - col_record.save + assert col_record.save col_record.references = 41 - col_record.save - c2 = ColumnName.find(col_record.id) + assert col_record.save + assert_not_nil c2 = ColumnName.find(col_record.id) assert_equal(41, c2.references) end diff --git a/activerecord/test/fixtures_test.rb b/activerecord/test/fixtures_test.rb index 888d743078..41fc666945 100755 --- a/activerecord/test/fixtures_test.rb +++ b/activerecord/test/fixtures_test.rb @@ -173,27 +173,45 @@ end if Account.connection.respond_to?(:reset_pk_sequence!) class FixturesResetPkSequenceTest < Test::Unit::TestCase fixtures :accounts + fixtures :companies - def test_resets_to_min_pk - Account.delete_all - Account.connection.reset_pk_sequence!(Account.table_name) + def setup + @instances = [Account.new(:credit_limit => 50), Company.new(:name => 'RoR Consulting')] + end + + def test_resets_to_min_pk_with_specified_pk_and_sequence + @instances.each do |instance| + model = instance.class + model.delete_all + model.connection.reset_pk_sequence!(model.table_name, model.primary_key, model.sequence_name) - one = Account.new(:credit_limit => 50) - one.save! - assert_equal 1, one.id + instance.save! + assert_equal 1, instance.id, "Sequence reset for #{model.table_name} failed." + end end - def test_create_fixtures_resets_sequences - # create_fixtures performs reset_pk_sequence! - max_id = create_fixtures('accounts').inject(0) do |max_id, (name, fixture)| - fixture_id = fixture['id'].to_i - fixture_id > max_id ? fixture_id : max_id + def test_resets_to_min_pk_with_default_pk_and_sequence + @instances.each do |instance| + model = instance.class + model.delete_all + model.connection.reset_pk_sequence!(model.table_name) + + instance.save! + assert_equal 1, instance.id, "Sequence reset for #{model.table_name} failed." end + end - # Clone the last fixture to check that it gets the next greatest id. - another = Account.new(:credit_limit => 1200) - another.save! - assert_equal max_id + 1, another.id + def test_create_fixtures_resets_sequences + @instances.each do |instance| + max_id = create_fixtures(instance.class.table_name).inject(0) do |max_id, (name, fixture)| + fixture_id = fixture['id'].to_i + fixture_id > max_id ? fixture_id : max_id + end + + # Clone the last fixture to check that it gets the next greatest id. + instance.save! + assert_equal max_id + 1, instance.id, "Sequence reset for #{instance.class.table_name} failed." + end end end end diff --git a/activerecord/test/migration_test.rb b/activerecord/test/migration_test.rb index 9c5c2285b7..3fa680b94a 100644 --- a/activerecord/test/migration_test.rb +++ b/activerecord/test/migration_test.rb @@ -362,6 +362,9 @@ if ActiveRecord::Base.connection.supports_migrations? ActiveRecord::Base.table_name_suffix = "" Reminder.reset_table_name assert_equal "schema_info", ActiveRecord::Migrator.schema_info_table_name + ensure + ActiveRecord::Base.table_name_prefix = "" + ActiveRecord::Base.table_name_suffix = "" end def test_proper_table_name @@ -398,17 +401,20 @@ if ActiveRecord::Base.connection.supports_migrations? ActiveRecord::Base.table_name_prefix = 'prefix_' ActiveRecord::Base.table_name_suffix = '_suffix' Reminder.reset_table_name + Reminder.reset_sequence_name WeNeedReminders.up assert Reminder.create("content" => "hello world", "remind_at" => Time.now) assert_equal "hello world", Reminder.find(:first).content WeNeedReminders.down assert_raises(ActiveRecord::StatementInvalid) { Reminder.find(:first) } + ensure ActiveRecord::Base.table_name_prefix = '' ActiveRecord::Base.table_name_suffix = '' Reminder.reset_table_name + Reminder.reset_sequence_name end - + def test_migrator_with_duplicates assert_raises(ActiveRecord::DuplicateMigrationVersionError) do ActiveRecord::Migrator.migrate(File.dirname(__FILE__) + '/fixtures/migrations_with_duplicate/', nil) |