[request] schema-aware disabling of referential integrity while using fixtures

Question

[request] schema-aware disabling of referential integrity while using fixtures

Opened this issue 6 years ago · 15 comments

The problem is that when using fixtures for testing, rails disables referential integrity by using table names only. This requires putting all necessary schema names into a schema_search_path in config/database.yml. This is quite a PITA to remember. It would be nice if it was done automagically.

Rails calls ActiveRecord::ConnectionAdapters::PostgreSQL::ReferentialIntegrity#disable_referential_integrity
that uses ActiveRecord::ConnectionAdapters::SchemaStatements#tables which returns table names only without schema name to be quoted in #disable_referential_integrity. I feel like something could be done by returning already enquoted (as in PG format() with %I) name within ActiveRecord::ConnectionAdapters::PostgreSQL::SchemaStatements#data_source_sql or somewhere there.

I'm not sure how doable it is or whether it is better to file a bug somewhere upstream, but I'll leave it here as an idea.

Answer 1 · 2019-02-08T01:37:31.000Z

Aparently, there is rails/rails#30846

Answer 2 · 2019-02-19T05:19:28.000Z

The fix seems reasonable, maybe I can push a new version with this improvement, just as one "good to have" since only in new Rails versions this might get fixed.

Does this work for you?

Answer 3 · 2019-02-25T21:27:23.000Z

It does thus far! I would use the one from 5.2 branch not to require newer ruby https://github.com/mlt/rails/commits/bug-30846-5.2 . It also uses reduced filter for schema names.

Answer 4 · 2019-02-26T05:24:33.000Z

Hold on! I need to include similar stuff to enable triggers back :-)

Answer 5 · 2019-03-05T02:24:13.000Z

I think it is good now! Same link, same branch.

Answer 6 · 2019-03-27T01:55:01.000Z

Hey @mlt, I'm finally with time to check this out. Was this fixed on Rails or do you think it's still a valid feature?

Answer 7 · 2019-03-28T17:59:50.000Z

It does not look like there is any action on this one upstream 😞 I personally still use it as it works for me. If you can make it into a monkey patch or something, that would be great! I just re-based both 5.2 and master branches.

Answer 8 · 2019-07-17T20:53:29.000Z

Are you working on this or would you rather prefer for me to attempt to create a PR for both my requests?

Answer 9 · 2019-07-18T04:34:10.000Z

Right now I'm not being able to work on this. I think it's worth creating a PR. I have a suggestion that might work and won't break anything.

# on app/models/user.rb
module User
  extend Torque::PostgreSQL::SchemaAwareness
  # Or something like this, just to make sure we manually set the base modules that should be used as schema
end

# then while building the table name, we can check if the class is under a module other than Object, and if that is a Module which also is schema_aware?

Answer 10 · 2019-07-18T23:05:51.000Z

I think what I have does not break anything… at least for me. I do not quite follow what is the point. The only downside of my approach that it unconditionally messes with triggers for all tables in all schemas. This might be heavy, I agree. But that is what Rails is doing for public only anyway.
I am not sure how other approach using schema. I personally have

module MyMod
  def self.table_name_prefix; 'my_schema.' end
...

So if we want to minimize number of tables we are messing with (for legacy DBs with tables not necessarily having Rails models) we can check whether self.table_name contains dot. Would not explicitly extending SchemaAwareness be an overkill? But once again this and/or your approach entail going over/enumerating all models. I feel like this is also heavy. I mean there might be a reason to just query pg_class and be done with it.

Answer 11 · 2019-07-19T01:20:24.000Z

The only problem with your approach is that it breaks some Rails rules, which we should try to follow to make sure that we definitely not gonna break anything. And I also think that SchemaAwareness it's way easier to control and change or add behaviors.

We need to use arel and table name quoting to guarantee the best compatibility. I know it can be an overkill, but there are rules and standards to follow. Besides, my idea with this GEM is that in the future, some of these features could be integrated on ActiveRecord with just a copy and paste (that's why it has this odd folder structure).

Answer 12 · 2019-07-19T20:36:53.000Z

I think you overestimate my understanding of Rails 😇 I get the whole point of doing things "right" though.

I think I created a monster 😞 It works… but it is ugly as hell. We need to weed out views. The only way I see it either through some marker in model class or filter out by querying the DB. Another can of worms is models using different connections within a schema-aware module.

Answer 13 · 2019-08-12T04:41:17.000Z

Also the other day I saw this https://stackoverflow.com/a/49584660/673826 but I didn't try. I wonder how neat it would be to use that.

Answer 14 · 2019-12-17T02:08:43.000Z

Rails 6 solves this problem, so I think there's no point to move forward with this feature. What do you think?

Answer 15 · 2020-08-08T02:52:59.000Z

Nope, can't confirm. It is still an issue with Rails 6.0.3.2. I've set up a sample repo to demonstrate the problem mlt/deleteme@e69df1c

bundle exec rake db:setup RAILS_ENV=test
bundle exec rails db:migrate RAILS_ENV=test
bundle exec rails test
bundle exec rails test

Upon succeeding re-runs, you'd see

WARNING: Rails was not able to disable referential integrity.

This is most likely caused due to missing permissions.
Rails needs superuser privileges to disable referential integrity.

    cause: 

Run options: --seed 34209

# Running:

E

Error:
TTest#test_should_pass:
ActiveRecord::InvalidForeignKey: PG::ForeignKeyViolation: ERROR:  update or delete on table "ts" violates foreign key constraint "ts_t_id_fkey" on table "ts"
DETAIL:  Key (t_id)=(1) is still referenced from table "ts".