Opened 10 years ago
Closed 10 years ago
#23434 closed Bug (fixed)
Incorrect parameter coercion for bool in Oracle backend
Reported by: | Josh Smeaton | Owned by: | Josh Smeaton |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | oracle backend orm |
Cc: | 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
The following code exists in backends.oracle.base.OracleParam:
# Oracle doesn't recognize True and False correctly in Python 3. # The conversion done below works both in 2 and 3. if param is True: param = "1" elif param is False: param = "0"
I believe the conversion should be to the integers 1
and 0
respectively, rather than strings. This hasn't manifested as a problem before, but it is causing problems when using user defined boolean annotations in #14030. The string "1" is being returned rather than the integer 1 so the field converters aren't receiving the type they expect (integers) and do no conversions.
The alternative to changing the code above would be to change the field converter to check for value in ("1","0")
and then do the relevant bool(int()) conversion, but I think that's working around the actual problem.
Happy to put a patch together. I just wanted to get some buy in that this needs fixing first, and that the approach above should be valid. I'm not sure whether OracleParam can work with just strings or not.
Change History (5)
comment:1 by , 10 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:2 by , 10 years ago
comment:3 by , 10 years ago
Easy pickings: | unset |
---|---|
Triage Stage: | Unreviewed → Ready for checkin |
Type: | Uncategorized → Bug |
Original code added by Anssi in [4db38cbf]. Hopefully he can either +1 this or say why it's wrong.
comment:4 by , 10 years ago
I don't recall why I used strings. As Oracle uses Number(1, 0) for BooleanFields, using 1 and 0 seems like the right choice here. +1
comment:5 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
https://github.com/django/django/pull/3190