Opened 13 years ago

Closed 11 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 13 years ago.
Patch that adds regression test and fix.
16134_fix2.diff (702 bytes ) - added by Hong Shuning 12 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 by Luke Plant, 13 years ago

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).

by RoboHamburger, 13 years ago

Attachment: 16134_fix.diff added

Patch that adds regression test and fix.

comment:2 by RoboHamburger, 13 years ago

Has patch: set

comment:3 by Luke Plant, 13 years ago

Triage Stage: AcceptedReady for checkin

LGTM.

comment:4 by Malcolm Tredinnick, 13 years ago

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 by Adam Nelson, 13 years ago

Patch needs improvement: set

comment:6 by Hong Shuning, 12 years ago

Cc: hongshuning@… added

by Hong Shuning, 12 years ago

Attachment: 16134_fix2.diff added

comment:7 by Hong Shuning, 12 years ago

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

comment:8 by antonino stefano borgia, 11 years ago

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

comment:9 by antonino stefano borgia, 11 years ago

Owner: antonino stefano borgia removed
Status: assignednew

comment:10 by Christopher Adams, 11 years ago

Owner: set to Christopher Adams
Status: newassigned

comment:11 by Christopher Adams, 11 years ago

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 by Christopher Adams, 11 years ago

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