ErwinM / acts_as_tenant

Easy multi-tenancy for Rails in a shared database setup.
MIT License
1.56k stars 264 forks source link

Add `global_records_identifier` configuration #332

Open marshluca opened 8 months ago

marshluca commented 8 months ago

we have a model using global records feature in acts_as_tenant gem

class Member < ApplicationRecord
  acts_as_tenant(:organization, has_global_records: true)
end

With a given tenant scope 1, It generates SQL like below:

SELECT `members`.* FROM `members` WHERE (`members`.`organization_id` = 1 OR `members`.`organization_id` IS NULL)

as the data increased, it spent about 2s, but after we changed the global records identifier to a number 3, the query time decreased to 70ms.

SELECT `members`.* FROM `members` WHERE `members`.`organization_id` IN (1, 3)

It looks like MySQL doesn't perform well on NULLvalues index. So I proposed this pull request to make the global records identifier configurable.

ActsAsTenant.configure do |config|
  config.global_records_identifier = 3
end
Development [1] demo(main)> ActsAsTenant.global_records_identifier
3
Development [2] demo(main)> Member.all.to_sql
"SELECT `members`.* FROM `members`"
Development [3] demo(main)> ActsAsTenant.with_tenant(Organization.first) { Member.all.to_sql }
"SELECT `members`.* FROM `members` WHERE `members`.`organization_id` IN (1, 3)"
Development [4] demo(main)> ActsAsTenant.with_tenant(Organization.last) { Member.all.to_sql }
"SELECT `members`.* FROM `members` WHERE `members`.`organization_id` IN (2, 3)"

It doesn't change the default behaviour, hope it is acceptable. Any feedback is welcome. Thanks.

excid3 commented 8 months ago

This seems reasonable to me. Should add some documentation for it though.

marshluca commented 8 months ago

This seems reasonable to me. Should add some documentation for it though.

@excid3 Thanks for the response! I've updated the documentation, please let me know if I missed anything.

marshluca commented 6 months ago

@excid3 I have made some updates, any feedback is welcome :)

This seems reasonable to me. Should add some documentation for it though.

marshluca commented 4 months ago

Any feedback on this PR? @excid3