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

Support DB Version in multi schema environment #370

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nick-benoit14
Copy link

We are using apartment for multi tenancy. We are having issues where this query gives back an inconsistent version because the records that come back from this query do not always have the same order and we are just using the first one. I think ideally this would only look at the public schema, but I figured this was maybe slightly lower touch and would at least improve our situation a fair bit.

@ZimbiX
Copy link
Member

ZimbiX commented Aug 9, 2022

Related: #294

[...] there are a mixture of references to tables with and without the hard-coded public. prefix [...]

I'm not familiar with apartment, but do I understand that you're trying to use multiple separate instances of Que on the same database, separating them using different schemas? This is not something that's supported - see the linked issue - and I'd be surprised if this was the only problematic query. I think I'd prefer to add support for specifying the schema.

@nick-benoit14
Copy link
Author

Thanks for the speedy reply. We are actually only using que in the public schema, due to some unfortunate side effects of apartment we have some schemas with a phantom que jobs table that doesn't actually get used. It does however however make the db_version call behave in bizarre and inconsistent ways due to the order of the results of the query coming back in no particular order.

Per schema support would be neat and potentially useful though. I don't fully grok how big of a lift full schema support is, but a potential stop along the way would be respecting the active search path in the db_version query. We run into these issues especially with migrations, largely due to the fact that when the db_version is set the search path is respected, and when the version is read the search path is disregarded. Of course the above code change does not address that, but is that an intermediate step that you think would make sense?

@fsateler
Copy link

as long as public is hardcoded, then the correct solution would be to add the schema filter:

INNER JOIN pg_namespace ON relnamespace = pg_namespace.oid
WHERE nspname = 'public'

Otherwise you might get a table from another schema

@nick-benoit14
Copy link
Author

Yeah, that is a much better solution. @ZimbiX If I implemented that would you be willing to merge? I think that would alleviate many of our headaches.

@fsateler
Copy link

I'm not familiar with apartment, but do I understand that you're trying to use multiple separate instances of Que on the same database, separating them using different schemas? This is not something that's supported - see the linked issue - and I'd be surprised if this was the only problematic query. I think I'd prefer to add support for specifying the schema.

Just for additional clarification. Apartment works by replicating the whole datamodel in several schemas. That does not mean all tables are used. In fact, what both @nick-benoit14 and I would want is that tenant_one.que_jobs is never used, only the public one. The reason for this is that it is quite rare that you want to have N distinct workers, where N is the number of clients: what you usually want is a single table for all tenants. This is correctly done today, because the schema public is hardocded in Que. Howver, this query might return information about some_other_schema.que_jobs which is wrong.

In summary, while specifying the schema might be nice, all this attempts to fix is the one case public is not hardcoded yet.

@ZimbiX
Copy link
Member

ZimbiX commented Sep 15, 2022

Yeah, that is a much better solution. @ZimbiX If I implemented that would you be willing to merge? I think that would alleviate many of our headaches.

In summary, while specifying the schema might be nice, all this attempts to fix is the one case public is not hardcoded yet.

Sure. I think that's probably better too. Then we'd have somewhere to specify the schema in this query for when we add support for specifying a custom schema for #294.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants