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

ActiveRecord::SchemaDumper removes prefixes and suffixes when ignoring tables. #9015

Closed
marcelokanzaki opened this issue Jan 21, 2013 · 8 comments

Comments

@marcelokanzaki
Copy link

When I have the following in my database:

  • some_table
  • my_prefix_some_table

and configure ActiveRecord::SchemaDumper.ignore_tables = ["some_table"]

The SchemaDumper ends up ignoring both tables, because it removes prefixes and suffixes before comparing tables names with the elements in the supplied array.

The result is that I never get the chance to ignore just one or the other table.

@rafaelfranca
Copy link
Member

Hmm, This is an interesting issue. It behaves this way because the schema.rb should not know about the prefix configuration. In this way it is easy to change the prefix/suffix configuration without need to dump the schema again.

I guess we can change these two lines https://github.com/rails/rails/blob/master/activerecord/lib/active_record/schema_dumper.rb#L73-74 to take in consideration only the table name with suffix.

Mind to open a pull request?

@marcelokanzaki
Copy link
Author

Don't mind at all my friend. And changing those two lines was exactly what I had in mind.
Thanks for taking the time to check.

@eval
Copy link
Contributor

eval commented Apr 29, 2013

I guess the situation described is problematic because of a more basic reason:
Without ignoring anything, the dump will contain create_table :some_table twice as table-names are stripped when dumped https://github.com/rails/rails/blob/master/activerecord/lib/active_record/schema_dumper.rb#L108

Fixing this would solve the ignoring-scenario as well, as stripped table-names are unique.
A solution might be to warn and/or skip the non-prefixed table when dumping?
@rafaelfranca ideas?

PS For completeness, the following is part of the configuration as well right?:

ActiveRecord::Base.table_name_prefix = 'my_prefix_'

@marcelokanzaki
Copy link
Author

I think we need to understand the motivations and use cases behind ActiveRecord::Base.table_name_prefix and ActiveRecord::SchemaDumper.ignore_tables.

For instance, I totally understand what @eval described above. Should the schema.rb file contain the create_table statements with the prefixes or just apply them at run-time?

For my use case I think it should be create_table prefix_table_name because the schema.rb should reflect the names in the database, otherwise it might be misleading (I'd like to know what you guys think).

Also, when using ActiveRecord::Base.table_name_prefix the prefix is also applied to the schema_migrations table (which I think its fine) but when Rails loads the schema in the test database, it checks for pending migrations, comparing the contents of the schema_migrations table on both databases (development and test) and it fails (probably because its not applying the prefixes to query the tables) with an ActiveRecord::PendingMigrationError.

@rafaelfranca
Copy link
Member

This issue has been automatically marked as stale because it has not been commented on for at least
three months.

The resources of the Rails team are limited, and so we are asking for your help.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

@nerdinand
Copy link
Contributor

I tried to reproduce this on master. It does not seem to be a problem anymore. See this test case:
https://gist.github.com/nerdinand/d67739425cd56c5427b4

repinel added a commit to repinel/rails that referenced this issue Jun 14, 2015
Since the issue rails#9015 was unexpected fixed by the PR rails#17697,
this commit makes sure that the cases reported by the issue are tested.
@timbreitkreutz
Copy link

I noticed this old ticket and did a bit of investigation.

  1. Like @nerdinand I verified that the bug no long seems to exist in master.
  2. I converted his test case gist to a unit test in schema_dumper_test.rb and packaged it into PR Test case for Issue #9015 - ignore_table and table_prefix at same time #22271.

If folks agree I suggest we merge in the test case and close this ticket...

@timbreitkreutz
Copy link

This was fixed in e23ec4c

timbreitkreutz pushed a commit to timbreitkreutz/rails that referenced this issue Nov 13, 2015
senny added a commit that referenced this issue Nov 17, 2015
…test-for-prefix-and-ignore

Test case for Issue #9015 - ignore_table and table_prefix at same time
@arthurnn arthurnn closed this as completed Dec 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants