-
Notifications
You must be signed in to change notification settings - Fork 965
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
Use raiseload
option for all queries
#13311
base: main
Are you sure you want to change the base?
Conversation
Question: Is there a way to approach this gradually? Perhaps via Warnings? Request: Can you implement a couple of the necessary fixes as examples? You mentioned that there might be some impacts on code readability. If its too egregious it may not be worth it. |
Let's pursue this once we've upgraded db libs. see: #13497 |
@miketheman Do you think we're ready to revisit this? |
OK! Let's say this is blocked on #15634. |
unblocked now that we can count queries emitted when loading a page via |
Rebased. Quite a number of test failures to work through here if anyone is feeling up for it 🙂 |
Sort of towards #3081.
This PR turns on the
raiseload
option by default for all queries, essentially making N+1 queries impossible in our codebase, and requiring explicit loads instead, rather than per-query as documented in https://docs.sqlalchemy.org/en/20/orm/queryguide/relationships.html#preventing-unwanted-lazy-loads-using-raiseload .Marking this as draft for now because there are a lot of test failures to address. I'm also not entirely sure yet if this also covers deferred columns as well, because that seems to be a separate setting: https://docs.sqlalchemy.org/en/20/orm/queryguide/columns.html#using-raiseload-to-prevent-deferred-column-loads