Examine ways to get rid of DB backend convert_values()
|Reported by:||Anssi Kääriäinen||Owned by:||Marc Tamlyn|
|Component:||Database layer (models, ORM)||Version:||master|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||yes|
The convert_values() method is responsible for converting database values to Python values. The conversion is used at least for aggregates, GIS and Oracle's query compiler.
The problem with current implementation is two-fold:
- The model fields have no way of specifying new converters. This is problematic for 3rd party fields. They would need to do some kind of a dynamic patch to the backend's convert_values() method if they need convert_values() support.
- The different backends have slightly differing implementation for the same cases.
In #13844 it was suggested that Field.to_python() method could be reused. It seems it does mostly the correct thing, but it is meant for validation, not converting DB values to Python. So, going that route would need at least some more investigation.
One possible solution is something like this: A field has convert_values() method. The field can (and likely will) call a backend specific converter method. So, a custom field could do the following:
- Add converter methods to backends it supports:
def psycopg2_converter(self, value): return value.lower() try: from django.db.backends.postgresql_psycopg2.operations import DatabaseOperations DatabaseOperations.convert_myfield_value = psycopg2_converter # Not sure if this exact syntax works. except ImportError: pass # Or, if there is need for just a generic converter: from django.db.backends import BaseDatabaseOperations BaseDatabaseOperations.convert_myfield_value = my_generic_converter
- The convert_values method of the field itself would be:
def convert_values(self, connection, value): return connection.convert_myfield_value(value)
A little complicated, but it is general. I think it would be fast enough.
Most fields could just do the conversion directly in field.convert_values() without messing with the DatabaseOperations at all. Core fields would not need to do any dynamic installation, as we could of course just install the converter into the source code.
The above proposal is just one way to achieve what is needed. I do think there is some need for improvement here, but I am not at all sure the suggestion in this post is the way forward.
Change History (7)
comment:1 Changed 4 years ago by
|Patch needs improvement:||unset|
|Triage Stage:||Unreviewed → Accepted|