bernardopires / django-tenant-schemas

Tenant support for Django using PostgreSQL schemas.
https://django-tenant-schemas.readthedocs.org/en/latest/
MIT License
1.46k stars 424 forks source link

Doesn't work with Postgresql extensions (hstore) #113

Closed owais closed 10 years ago

owais commented 10 years ago

Tenant models fail to sync if they try to use postgresql's hstore field. It seems hstore functions are not visible to the sync command. The hstore extensions are installed in the public schema.

bernardopires commented 10 years ago

Hi,

I don't think this error has anything to do with this app. The only thing this app is doing is changing the search_path. This might just be the insight we need to solve this problem. AFAIK postgresql's extensions are not global but rather installed to a particular schema. It may be failing because hstore doesn't exist on the schema you're trying to sync. Hope this helps.

owais commented 10 years ago

@bernardopires AFAIK, extensions are not installed to every schema. Usually, they are installed to the public schema and they become available if you make public visible using search_path. With sync_schemas --tenant, the public schema is not added to the search_path hence the extension functions are not visible.

If making public visible to sync_schemas --tenant is not feasible then a solution would be to create a dedicated schema to hold all extensions and add it to search_path by default.

It could be configurable using a setting like PG_EXTRA_SEARCH_PATHS.

owais commented 10 years ago

Closed accidentally, sorry.

owais commented 10 years ago

@bernardopires So is my hypothesis right? If yes, would what I've proposed be a viable solution? I'm willing to work on it and submit a pull request if we agree on a solution.

bernardopires commented 10 years ago

Hi owais,

yes, you're correct, sync_schemas --tenant will not work properly if the public schema is included. I was looking to avoid adding more settings to the app, but I'm not seeing another way around. Supporting PostgreSQL extensions such as hstore also seem like a very reasonable feature.

Could you please work on a patch? I'll gladly accept it! Thanks a lot in advance!

owais commented 10 years ago

@bernardopires Sure, I'll cook something up tomorrow and send you a pull-request.

philfriesen commented 10 years ago

Hi Bernardo & Owais:

Thank you for your work on this! After pulling Owais' changes, I would get errors when tables were created for models using the hstore data type, even though I had created a schema named 'extensions', created the hstore extension in it, and then added Owais' PG_EXTRA_SEARCH_PATHS = ['extensions'] to settings.py.

I found a suggestion at http://stackoverflow.com/questions/12986368/installing-postgresql-extension-to-all-schemas to install the extension in the pg_catalog schema, which makes all functions available to all other schemas. Then everything worked like a charm!

If I take this approach, I don't need to use PG_EXTRA_SEARCH_PATHS in settings.py.

Regardless, thanks for pointing me in the right direction.

--Phil.

owais commented 10 years ago

Nice find @philfriesen, this simplifies things a bit. So does PG_EXTRA_SEARCH_PATHS have any significance now? I think it can be useful but can't put my finger on an exact use case. Should we revert the change and add the pg_cataglog tip to docs?

owais commented 10 years ago

"I would strongly recommend creating a new unaccent schema and creating it in that, then adding that schema to your search_path. Cleaner to avoid messing with the system catalogs, and putting it in a new schema doesn't stop you having it on the search path." - http://stackoverflow.com/questions/12986368/installing-postgresql-extension-to-all-schemas#comment23040069_16128622

philfriesen commented 10 years ago

Hi @owais . We have complete control of our DB for our apps, but I can see other shops that might want a common schema when public is not available. Also, our future plans for scaling out dozens of app servers include the possibility of grouping tenant schemas, where each group would need a common group schema, say group1, group2, etc. So we would want to conditionally add a common schema named groupX, where tenants1-10 might belong to group1, 11-20 to group2, etc. So adding a schema to the path is a good idea, but not for sharing extensions. I simply do not know how to make extensions available to all other schemas except using the pg_catalog schema.

owais commented 10 years ago

@philfriesen I don't think using a dedicated schema for holding extensions is such a bad idea. I don't see any downsides to it, in fact by default the public schema does hold all the extensions you create.

Your usecase sounds pretty convincing but it would probably take quite some amount of work to support multiple schemas per tenant. I'm willing to contribute to the feature :)

I was thinking if it would be best to make this feature available on the queryset + model manager. I mean something like this,

MyModel.objects.using_schema('schema1', 'schema2').get(...)
philfriesen commented 10 years ago

@owais I would much prefer a dedicated schema for holding extensions instead of commandeering the pg_catalog schema. I'm very, very new to Django. Hopefully by the time we need the feature I will understand enough to collaborate with you.

I would prefer being able to set some property so the using_schema('schema1', 'schema2') method would be implemented on the ObjectManager itself somehow so the developer would not have to treat a particular model differently. The developer would still use MyModel.objects.get(...) and the ObjectManager would add the using_schema('schema1', 'schema2') logic. Or whatever gets the connection would set the schema path.

Please forgive my ignorance of existing Django functionality. It's a beautiful thing, but there's lots to learn. I'm sure I've misstated things, but I'm sure you understand what I'm aiming for. Thanks!

davidmir commented 9 years ago

Hi,

I created a schema 'extensions' that has installed postgis. I also added in the settings files PG_EXTRA_SEARCH_PATHS = ['extensions'].

However when I run the migration it gives me the following error:

'django.db.utils.ProgrammingError: type "geometry" does not exist'

If I install postgis extension in one of the tenant schemas this error no longer occurs and all migrations run successfully.

This means that he is ignoring the schema added in PG_EXTRA_SEARCH_PATHS?

Thanks in advance

philfriesen commented 9 years ago

Just add all extensions to the “public” schema in Postgres. We did this for hstore and it works great! --Phil.

From: davidmir [mailto:notifications@github.com] Sent: Monday, June 01, 2015 11:13 AM To: bernardopires/django-tenant-schemas Cc: Friesen, Phil Subject: Re: [django-tenant-schemas] Doesn't work with Postgresql extensions (hstore) (#113)

Hi,

I created a schema 'extensions' that has installed postgis. I also added in the settings files PG_EXTRA_SEARCH_PATHS = ['extensions'].

However when I run the migration it gives me the following error:

'django.db.utils.ProgrammingError: type "geometry" does not exist'

If I install postgis extension in one of the tenant schemas this error no longer occurs and all migrations run successfully.

This means that he is ignoring the schema added in PG_EXTRA_SEARCH_PATHS?

Thanks in advance

— Reply to this email directly or view it on GitHubhttps://github.com/bernardopires/django-tenant-schemas/issues/113#issuecomment-107570415.

davidmir commented 9 years ago

It gives the same error with the extension in the public schema.

owais commented 9 years ago

Can you try to execute a query in postgresql shell that uses postgis extension after setting search_path to extensions? If it works, then the bug is in tenant-schemas.

@philfriesen, it is not recommended to add extensions to public. They'll not be always available. For example, when migrating private schemas only or when you want the query to run only on a private schema aka seach_path=your_private_schema. Any calls to hstore functions will fail at such times. It is better to create a dedicated schema for extensions and always keep it on search_path.

On Mon, Jun 1, 2015 at 9:11 PM davidmir notifications@github.com wrote:

It gives the same error with the extension in the public schema.

— Reply to this email directly or view it on GitHub https://github.com/bernardopires/django-tenant-schemas/issues/113#issuecomment-107593031 .

davidmir commented 9 years ago

It works. Now the PG_EXTRA_SEARCH_PATHS does not work in the current version, gives: django.core.exceptions.AppRegistryNotReady: Models aren't loaded yet.

davidmir commented 9 years ago

Has anyone used the PostGIS in version 1.5.5?