Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fix sequence name with abstract classes. #3851

Merged
merged 1 commit into from Dec 4, 2011

Conversation

Projects
None yet
3 participants
Contributor

ebeigarts commented Dec 4, 2011

With oracle-enhanced this test started to fail (I think it was after this commit):

  1) Error:
test_validate_straight_inheritance_uniqueness(UniquenessValidationTest):
OCIError: ORA-02289: sequence does not exist
    /Users/beigaedg/.rvm/gems/ruby-1.9.2-p290@rails/gems/ruby-oci8-2.0.6/lib/oci8/oci8.rb:288:in `exec'
    /Users/beigaedg/.rvm/gems/ruby-1.9.2-p290@rails/gems/ruby-oci8-2.0.6/lib/oci8/oci8.rb:116:in `exec'
    /Users/beigaedg/code/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced_oci_connection.rb:469:in `exec'
    /Users/beigaedg/code/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced_oci_connection.rb:172:in `select'
    /Users/beigaedg/code/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced_connection.rb:83:in `select_one'
    /Users/beigaedg/code/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced_connection.rb:89:in `select_value'
    /Users/beigaedg/code/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb:761:in `next_sequence_value'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/relation.rb:47:in `insert'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/persistence.rb:316:in `create'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/timestamp.rb:57:in `create'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/callbacks.rb:268:in `block in create'
    /Users/beigaedg/code/rails/activesupport/lib/active_support/callbacks.rb:393:in `_run_create_callbacks'
    /Users/beigaedg/code/rails/activesupport/lib/active_support/callbacks.rb:81:in `run_callbacks'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/callbacks.rb:268:in `create'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/persistence.rb:297:in `create_or_update'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/callbacks.rb:264:in `block in create_or_update'
    /Users/beigaedg/code/rails/activesupport/lib/active_support/callbacks.rb:393:in `_run_save_callbacks'
    /Users/beigaedg/code/rails/activesupport/lib/active_support/callbacks.rb:81:in `run_callbacks'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/callbacks.rb:264:in `create_or_update'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/persistence.rb:37:in `save'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/validations.rb:50:in `save'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/attribute_methods/dirty.rb:22:in `save'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/transactions.rb:241:in `block (2 levels) in save'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/transactions.rb:295:in `block in with_transaction_returning_status'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:190:in `transaction'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/transactions.rb:208:in `transaction'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/transactions.rb:293:in `with_transaction_returning_status'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/transactions.rb:241:in `block in save'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/transactions.rb:252:in `rollback_active_record_state!'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/transactions.rb:240:in `save'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/base.rb:533:in `create'
    /Users/beigaedg/code/rails/activerecord/test/cases/validations/uniqueness_validation_test.rb:269:in `test_validate_straight_inheritance_uniqueness'
    /Users/beigaedg/.rvm/gems/ruby-1.9.2-p290@rails/gems/mocha-0.9.12/lib/mocha/integration/mini_test/version_142_to_172.rb:27:in `run'
    /Users/beigaedg/code/rails/activesupport/lib/active_support/testing/setup_and_teardown.rb:35:in `block in run'
    /Users/beigaedg/code/rails/activesupport/lib/active_support/callbacks.rb:415:in `_run_setup_callbacks'
    /Users/beigaedg/code/rails/activesupport/lib/active_support/callbacks.rb:81:in `run_callbacks'
    /Users/beigaedg/code/rails/activesupport/lib/active_support/testing/setup_and_teardown.rb:34:in `run'

I also added a new test case which failed also on PostgreSQL.

PostgreSQL:

  1) Error:
test_sequence_name_with_abstract_class(BasicsTest):
NoMethodError: undefined method `split' for nil:NilClass
    /Users/beigaedg/code/rails/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb:897:in `default_sequence_name'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/base.rb:736:in `reset_sequence_name'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/base.rb:725:in `sequence_name'
    /Users/beigaedg/code/rails/activerecord/lib/active_record/base.rb:727:in `sequence_name'
    ./test/cases/base_test.rb:1609:in `test_sequence_name_with_abstract_class'
    /Users/beigaedg/.rvm/gems/ruby-1.9.2-p290@rails/gems/mocha-0.9.12/lib/mocha/integration/mini_test/version_142_to_172.rb:27:in `run'
    /Users/beigaedg/code/rails/activesupport/lib/active_support/testing/setup_and_teardown.rb:35:in `block in run'
    /Users/beigaedg/code/rails/activesupport/lib/active_support/callbacks.rb:415:in `_run_setup_callbacks'
    /Users/beigaedg/code/rails/activesupport/lib/active_support/callbacks.rb:81:in `run_callbacks'
    /Users/beigaedg/code/rails/activesupport/lib/active_support/testing/setup_and_teardown.rb:34:in `run'

Oracle:

  1) Failure:
test_sequence_name_with_abstract_class(BasicsTest) [./test/cases/base_test.rb:1611]:
<"projects_seq"> expected but was
<"_seq">.
Member

jonleighton commented Dec 4, 2011

Thanks for reporting the bug. I think perhaps we should be using base_class here. E.g. something like:

def sequence_name
  if base_class == self
    @seqence_name ||= reset_sequence_name
  else
    (@sequence_name ||= nil) || base_class.sequence_name
  end
end

Could you look into that please?

Contributor

ebeigarts commented Dec 4, 2011

Yes, it works, I also updated my pull request.

@jonleighton jonleighton added a commit that referenced this pull request Dec 4, 2011

@jonleighton jonleighton Merge pull request #3851 from ebeigarts/fix_sequence_name
Fix sequence name with abstract classes.
4ded0dd

@jonleighton jonleighton merged commit 4ded0dd into rails:master Dec 4, 2011

This seems to break code where an inheritance tree defines both a table_name_prefix and defines a sequence pattern for all models inheriting from this tree.

Given:

class A < B
end

class B < ActiveRecord::Base
  self.abstract_class = true
  self.table_name_prefix = 'core.'
  self.sequence_name = :autogenerated
end

The queries will work correctly, but because of the behavior of base_class, when the connection asks for the sequence_name, base_class returns A and not B. A then resets the sequence_name and it is no longer pulled from the super, instead, you get a sequence name 'core.a_seq'

Seems we are missing a test case where the sequence_name is set in the base class and is retained in the subclass.

Are there any suggestions for work arounds?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment