Opened 3 years ago

Closed 2 years ago

#19197 closed Bug (fixed)

BaseDatabaseOperations: convert_values raise error on None data type conversions

Reported by: maxi Owned by: maxirobaina@…
Component: Database layer (models, ORM) Version: 1.4
Severity: Normal Keywords:
Cc: maxirobaina@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

convert_values method of BaseDatabaseOperations, make a explicit convertion to int type on IntegerField (or derived). The value to convert is not checked, then if value is None a TypeError is raised.

TypeError: int() argument must be a string or a number, not 'NoneType'

Similar case on float convertion.

Change History (5)

comment:1 Changed 3 years ago by akaariai

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Could you provide some sample code (or better yet, test case) to show this problem?

comment:2 Changed 3 years ago by maxi

Ok, I'll try to explain this:

  1. Maybe it only happens with database engines that override DatabaseOperations.convert_values and use this method in SQLCompiler.resolve_columns (I'm working on Firebird engine but you can see at Oracle implementation).
  1. The specific DatabaseOperations.convert_values must to call super from base class (Oracle implementation doesn't do that), then

def convert_values(self, value, field):

value = super(DatabaseOperations, self).convert_values(value, field)
...
# others values conversions
return value

  1. The problem is exposed on model_fields test app, BigIntegerFieldTests, test_types method

I hope I have made this sufficiently clear.

Let me know if you need more information.

comment:3 Changed 3 years ago by akaariai

  • Triage Stage changed from Unreviewed to Accepted

OK. Marking as accepted.

comment:4 Changed 2 years ago by akaariai

Patch available at https://github.com/akaariai/django/compare/ticket_19197

There is a fix for completely unrelated issue in one commit - backends tests seem to leak connections if ran alone on postgresql. This ticket's issue is fixed without tests, the base convert_values() has been doing a different thing than for example sqlite3's convert_values(), so writing a test for just base convert_values() seemed wrong.

In general the situation with convert_values() and Field.to_python is somewhat messy. It would be nice to clean that up but not this ticket's problem...

comment:5 Changed 2 years ago by Anssi Kääriäinen <akaariai@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 1caa483914edf851137b3c1e39f4364772bd46c9:

[1.5.x] Fixed #19197 -- fixed convert_values() for nullable numeric fields

Cleaned up the implementation of base convert_values() a little, and
made sure it accepts None as a value for numeric fields.

There are no tests attached. The reason is that not all of the
convert_values() accept None as a value for numeric fields (for example
sqlite3.convert_values()).

The reason the base convert_values() needs to accept None is that this
situation might arise in custom compilers for 3rd party backends. It
is easy to keep the convert_values() working, so lets do that.

Backpatch of 12a96bfa263d514884ef11009913b2f8bb163472

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