Opened 3 days ago
Last modified 9 hours ago
#36048 assigned Bug
CompositePrimaryKey lookup guards raise NotSupportedError instead of a more appropriate exception
Reported by: | Jacob Walls | Owned by: | Jacob Walls |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Release blocker | Keywords: | |
Cc: | Jacob Walls, Csirmaz Bendegúz, Simon Charette | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Django mostly(*) raises NotSupportedError
as documented in PEP 249, to signify that a database feature is missing. Instead, some of the CompositePrimaryKey
sanity checks raise regardless of db feature flags. Given that, I'm suggesting that ValueError
etc. would be more appropriate in these two locations:
Our guidance says "specific backends":
This is used on specific backends to rule out known expressions that have problematic or nonexistent implementations. If the expression has a known problem, the backend should raise NotSupportedError.
Essentially the inverse of #28665. Marking as a release blocker for now to ensure we consider this before finalizing the feature.
*: The handful of places we're raising NotSupportedError
regardless of db feature flags include:
- Query.check_filterable
- QuerySet._not_support_combined_queries
- QuerySet.get
- Field.slice_expression
- ResolvedOuterRef.resolve_expression
I'm not suggesting to do anything about those at the moment.
Change History (3)
comment:1 by , 37 hours ago
Cc: | added |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 9 hours ago
Triage Stage: | Accepted → Ready for checkin |
---|
I think I agree