Add Support For Foreign Keys Across Schemas #397
Comments
|
I think that isn't part of apartment. Postgres supports FKs across schemas. |
|
In my opinion, the apartment use migration generate SQL to implement multi-tenant feature. But the Rails will generate foreign keys when migration define So, the problem is migration contains foreign key SQL but not convert multi tenant syntax, and cause the migration errors. I think this feature is still necessary, otherwise disable all foreign key in migrations. |
|
i don't think rails supports things like cross-schema foreign keys or composite foreign keys. if you want to use these features i believe you'll have to drop to SQL in your migrations and use i don't think this is an apartment responsibility, so closing for now. |
|
Frustrating to see all these closed, would you accept a pull request? |
|
@robertjwhitney didn't mean to frustrate; just trying to do a clear up so we can see which are actualy issues. my impression, supported by your link, was that whatever solution for cross-schema foreign keys there is that it would need to be at the rails level (migration/schema). if you believe there's something we can do in apartment, let me know and i'll reopen! |
|
Hey guys, I had opened rails issue for the cross-schema foreign key. It got closed with comment saying it matter of Apartment gem. Can someone please look into it - rails/rails#28654. |
|
feel free to suggest a solution! :) |
|
Hey guys. |
|
The solution found by me class AddForeignKeysForYearAssignments < ActiveRecord::Migration[5.1]
def up
execute <<~SQL
ALTER TABLE #{schema}.year_assignments
ADD CONSTRAINT fk_year_assignments_lessons FOREIGN KEY (lesson_id)
REFERENCES public.lessons(id)
SQL
end
def down
execute <<-SQL
ALTER TABLE #{schema}.year_assignments
DROP CONSTRAINT fk_year_assignments_lessons
SQL
end
private
def schema
Apartment::Tenant.current
end
end |
|
Hey guys, let me help those who are stuck on this question. The problem: When you create a tenant, Apartment will do a db:schema:load (not running migrations as @arturtr presumed) to create tables/columns and the add_foreign_key without explicit reference to public will generate a fk pointing to table inside your private tenant. The result is: FK error on all your querys, because in fact postgres is trying to access a register that doesn't exist. If you try to add reference on schema.rb manually, it will work and tenant will create column pointing to public. But as you know, it isn't sustainable. The solution:
Done, now you have everything working and pointing to the correct schemas. Apartment '2.2.0' Att, |
|
My solution works for us because we create new tenants manually with creating DB with |
|
@marcosvieiraftw either I don't understand your solution or it just removes db-level foreign key constraint and doesn't offer anything in return. Passing |
|
@mikecmpbll this is an issue Apartment should handle. As per DmytroVasin comment on rails/rails#28654, Postgres will omit Now, since Apartment runs migrations in other schemas, it should be detecting when I will look into how hard would it be to fix it but given that I have zero knowledge of the internals of ActiveRecord I guess I will fail at this. |
|
The best solution I came up with after more than 4 hours spent on this. Tested only on Rails 5.1.6.1. If you really really want to use foreign keys from tenanted table to the shared one you can define a migration like this: with any normal options, you would use. This migration will work correctly both on shared schema and tenant schemas as long as you only migrate the database. If you try to load schema from You could avoid loading schema but in larger teams, it will become a nuisance with people trying to use command they are used to and getting weird errors. You could override The other solution would be to mimic what foreign key does in terms of referential integrity and add a |
|
I have run into the same issue with foreign keys on apartment excluded tables (public). After some try/error I got to the same conclusion than @mszyndel. The way to go is to not create the FK and handle data integrity at the application level. I'm using Existing tenants will migrate fine, however, it does not work for newly created tenants from the structure.sql file. Reasons are, the SQL will have something like I have thought about a possible solution, use a different schema than public for shared data and leave public just for the SQL structure, in the end, it feels too much complexity for the benefit. I'm planning to move from apartment gem to column based tenancy within a single schema. |
|
Hello guys. After struggling with this problem for a long time, i found a way to make rails works with cross-schema references, using the usual rails foreign key relations. As long as you dont have the duplicate of the tables in the current schema, rails will fallback to the public schema The problem: even if the you set the Model as an Excluded Model, Apartment still creates an empty table for it in every tenant schema, when it runs the migrations. The solution: I've created a migration that drops all tables related to the excluded models, in every schema but the public one. This way we get rid of the useless empty tables, and rails automatically fallback to the public schema when it does not find the relation in the current schema. class FixCrossSchemaForeignKeys < ActiveRecord::Migration[5.2]
def up
unless schema == "public"
Apartment.excluded_models.each do |model|
begin
ActiveRecord::Base.connection.execute("DROP TABLE IF EXISTS #{schema}.#{model.downcase.pluralize} CASCADE")
rescue ActiveRecord::StatementInvalid => e
puts "Moving foward from the following error:"
puts e.message
end
end
end
end
def down
raise ActiveRecord::IrreversibleMigration
end
private
def schema
Apartment::Tenant.current
end
endThe drawback: It is an irreversible migration, and it only works for the excluded models you defined before this migration. If you need to make another cross-schema reference in the future, you would need to do another migration like this one. @mikecmpbll |
|
I'm afraid this workaround won't do the job for new tenants either as
tenant creation duplicates public schema.
You can define foreign keys using the form 'public.tablename' in the
migration. It will work for existing tenants at migration time but new ones
will replace 'public' with the new tenant schema name.
El jue., 29 ago. 2019 17:44, Rayan Chaves de Castro <
notifications@github.com> escribió:
… Hello guys.
After struggling with this problem for a long time, i found a way to make
rails works with cross-schema references, using the usual rails foreign key
relations.
As long as you dont have the duplicate of the tables in the current
schema, rails will fallback to the public schema
The problem is: even if the you set the Model as an Excluded Model,
Apartment still creates an empty table for it in every tenant schema, when
it runs the migrations.
The solution: Create a migration that drops all tables related to the
excluded models, in every schema but the public one. This way you get rid
of the useless empty tables, and rails automatically fallback to the public
schema when it does not find the relation in the current schema.
`
class FixCrossSchemaForeignKeys < ActiveRecord::Migration[5.2]
def up
unless schema == "public"
Apartment.excluded_models.each do |model|
begin
ActiveRecord::Base.connection.execute("DROP TABLE IF EXISTS
#{schema}.#{model.downcase.pluralize} CASCADE")
rescue ActiveRecord::StatementInvalid => e
puts "Moving foward from the following error:"
puts e.message
end
end
end
end
def down
raise ActiveRecord::IrreversibleMigration
end
private
def schema
Apartment::Tenant.current
end
end
`
This do have a drawback. It is an irreversible migration, and it only
works for the excluded models you defined before this migration. If you
need to make another cross-schema reference in the future, you would need
to do another migration like this one.
@mikecmpbll <https://github.com/mikecmpbll>
Apartment could try to implement this by default, preventing the tenant
schemas to create tables from excluded models.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#397?email_source=notifications&email_token=AAAINDFIX6M2KWCRKO2COWTQG74FNA5CNFSM4C625WT2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5PDO6I#issuecomment-526268281>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAINDBIB54TRIOSY665E7LQG74FNANCNFSM4C625WTQ>
.
|
|
Unfortunately trying to add Only adds Which still causes the I like @marcosvieiraftw's suggestion above to enforce the foreign key in the Model class |
|
My requirements demand data integrity starting at the database level. This was a tricky issue. FWIW, I managed to get it working with foreign keys. Hope this helps. # migrations
class CreateApplicationSchema < ActiveRecord::Migration[7.0]
def change
execute "CREATE SCHEMA IF NOT EXISTS application"
end
end
class CreateUsers < ActiveRecord::Migration[7.0]
def change
create_table "application.users" do |t|
t.timestamps
end
end
end
class CreatePosts < ActiveRecord::Migration[7.0]
def change
create_table :posts do |t|
t.bigint :user_id
t.timestamps
end
add_foreign_key :posts, "application.users", column: :user_id
end
endAnd configured Apartment: # initializer
Apartment.configure do |config|
config.excluded_models = []
endAs an added bonus, I don't have to configure # model
class User < ApplicationRecord
self.table_name = "application.users"
end
class Post < ApplicationRecord
belongs_to :user
endRails and Apartment however know nothing about the application schema, so in essence ❯ pg_dump -d mbb_development -N '(public|tenant*)' -s > db/structure.sqlI still have to enhance some db:tasks to load Hopefully I haven't forgotten anything. If it doesn't work, let me know. Patching apartment would probably be more efficient. In action with FactoryBot: 3.1.0 » Apartment::Tenant.create('test')
3.1.0 » Apartment::Tenant.switch!('test')
3.1.0 » Post.create(title: 'testing', user: create(:user))
3.1.0 » Post.all
=> [
[0] #<Post:0x00007ff2b81dc380>
]
3.1.0 » Post.first.user
=> #<User:0x00007fd5a78da688>
3.1.0 » Apartment::Tenant.switch!('public')
3.1.0 » Post.all
=> [] |
Is this on the roadmap at all? Postgres allows for it, I've seen a couple of issues but no real solutions, one of them even has a solution that doesn't seem to work in it. I'm guessing that it is a serious re-write, but having all users in one schema is probably pretty common, any thoughts?
The text was updated successfully, but these errors were encountered: