Opened 4 months ago
Last modified 3 months ago
#36385 assigned Cleanup/optimization
Simplify implementation of filtering in QuerySet.
Reported by: | Nick Pope | Owned by: | Nick Pope |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Simon Charette | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | yes |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
Some of the code for filtering QuerySet is overly complex which makes it hard to follow:
- Methods that are nearly identical, e.g.
QuerySet._clone()
vsQuerySet._chain()
andQuery.clone()
vsQuery.chain()
- These used to do more, but now are not significantly different such that they're worth keeping separate.
- Private API like
Queryset.complex_filter()
which exists for a specific case but can be handled using public APIs - Flags such as
_defer_next_filter
and_sticky_filter
that affect the next call toQuerySet.filter()
- Passing around deconstructed pieces instead of building a
Q()
object as early as possible - Not using the
klass
argument toQuery.chain()
and altering__class__
manually
We should be able to simplify this, removing a number of flag attributes and methods in the process.
Change History (3)
comment:1 by , 4 months ago
Has patch: | set |
---|
comment:2 by , 4 months ago
Cc: | added |
---|---|
Triage Stage: | Unreviewed → Accepted |
Hello Nick, thank you for the ticket and carefully crafted PR. After a quick look this seems like a positive cleanup, so I'm accepting pending feedback from Simon in the PR.
comment:3 by , 3 months ago
Needs documentation: | set |
---|---|
Patch needs improvement: | set |
The patch itself is great and a very welcomed cleanup but I suggest we defer the complete removal of the internal methods with deprecation shims and mentions in the release notes as it will inevitably break code out there.
Marking as needing improvement for this reason.
PR