Opened 6 years ago

Closed 3 years ago

#16134 closed Bug (fixed)

django.db.models.fields.Field instance can not be compared with None

Reported by: amcharg@… Owned by: Christopher Adams
Component: Database layer (models, ORM) Version: 1.3
Severity: Normal Keywords:
Cc: hongshuning@…, stefano.borgia@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: yes UI/UX: no

Description

Instances of django.db.models.fields.Field can not be compared with None. Eg: field == None

Repro: Compare a field instance with None. This fails since cmp assumes the other operand is a field instance and eq and ne are not defined.

Attachments (2)

16134_fix.diff (1.3 KB) - added by RoboHamburger 6 years ago.
Patch that adds regression test and fix.
16134_fix2.diff (702 bytes) - added by Hong Shuning 4 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 6 years ago by Luke Plant

Triage Stage: UnreviewedAccepted

Accepted. (BTW, the standard Python idiom for comparing against None is 'x is None' or 'x is not None', rather than == or !=, because it is more efficient and unambiguous. But the ticket is valid nonetheless).

Changed 6 years ago by RoboHamburger

Attachment: 16134_fix.diff added

Patch that adds regression test and fix.

comment:2 Changed 6 years ago by RoboHamburger

Has patch: set

comment:3 Changed 6 years ago by Luke Plant

Triage Stage: AcceptedReady for checkin

LGTM.

comment:4 Changed 5 years ago by Malcolm Tredinnick

Triage Stage: Ready for checkinAccepted
UI/UX: unset

Hmm... I'm a little unclear on the use-case for this. Instantiating fields is internal to Django -- normal models expose Python attributes and the Field instances are hidden in the Meta class (which is opaque). Making this change "just because we can" isn't necessary. What's the normal code pattern that triggers a problem here?

comment:5 Changed 5 years ago by Adam Nelson

Patch needs improvement: set

comment:6 Changed 4 years ago by Hong Shuning

Cc: hongshuning@… added

Changed 4 years ago by Hong Shuning

Attachment: 16134_fix2.diff added

comment:7 Changed 4 years ago by Hong Shuning

I modify the test case according to mtredinnick's propose based on dev branch. The test case succeed without modify any code.

comment:8 Changed 4 years ago by antonino stefano borgia

Cc: stefano.borgia@… added
Owner: changed from nobody to antonino stefano borgia
Status: newassigned

comment:9 Changed 4 years ago by antonino stefano borgia

Owner: antonino stefano borgia deleted
Status: assignednew

comment:10 Changed 3 years ago by Christopher Adams

Owner: set to Christopher Adams
Status: newassigned

comment:11 Changed 3 years ago by Christopher Adams

Simon Charette's commit at 5cbfb48b (intended to fix #17851) actually resolves this issue also. This commit made its way into stable/1.5.x but is not yet in stable/1.4.x. As this is not a security issue, both Russell Keith-Magee and Andrew Godwin tell me not to bother with a pull request.

comment:12 Changed 3 years ago by Christopher Adams

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.
Back to Top