Opened 5 weeks ago

Closed 4 weeks ago

#36048 closed Bug (fixed)

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 (4)

comment:1 by Sarah Boyce, 5 weeks ago

Cc: Csirmaz Bendegúz Simon Charette added
Triage Stage: UnreviewedAccepted

I think I agree

comment:2 by Jacob Walls, 5 weeks ago

Has patch: set

comment:3 by Sarah Boyce, 5 weeks ago

Triage Stage: AcceptedReady for checkin

comment:4 by Sarah Boyce <42296566+sarahboyce@…>, 4 weeks ago

Resolution: fixed
Status: assignedclosed

In 46b3e7dd:

Fixed #36048 -- Preferred ValueError to NotSupportedError for composite pk sanity checks.

These checks are not backend-dependent.

Note: See TracTickets for help on using tickets.
Back to Top