-
Notifications
You must be signed in to change notification settings - Fork 50
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
remove view from mysql backend #59
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good? Thanks for adding the extra index: I'd expect that to actually have measurable impact.
The view_queue is not materialized, and thus functions as syntactic sugar for a complex query. Furthermore, our infrastructure works poorly with views. Therefore, this usage adds a significant maintainership burden for little gain. This commit removes the view from NanoMDM code paths, but it still exists in the schema. This is purely for convenience and interactive users. Furthermore, I added an index used by the queue query, which should have existed for the view. This should improve performance. Testing: * Tested migration - see the schema migration script * Tested base functionality * Passes new integration test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm and thanks! Apologies for taking so long! For some reason I thought this was a WIP. Cheers!
ON r.command_uuid = q.command_uuid AND r.id = q.id | ||
WHERE q.id = ? | ||
AND q.active = 1 | ||
AND (r.status IS NULL OR (r.status = 'NotNow' AND NOT ?)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clever! I hadn't thought about using a bool
value as a query param.
The view_queue is not materialized, and thus functions as syntactic
sugar for a complex query. Furthermore, our infrastructure works poorly
with views. Therefore, this usage adds a significant maintainership
burden for little gain.
This commit removes the view from NanoMDM code paths, but it still
exists in the schema. This is purely for convenience and interactive
users.
Furthermore, I added an index used by the queue query, which should have
existed for the view. This should improve performance.
Testing:
depends on #58
fixes #53 for mysql backend