Model fields: calling `get_prep_value` from `get_db_prep_value`

Django Internals ORM
1

Hello everyone,

I’ve been reviewing the PR for this ticket. This is about how JSONField is not calling get_prep_value when adapting the field content in get_db_prep_value.

I’ve been doing some reading of django/db/models/fields/*.py and I’ve noticed a few things, that I wanted to discuss with all of you to gain a better understanding of whether any of these items are worth creating tickets for and if they require improvement work to enhance consistency and predictability.

Considering the fact that the base class Field provides a get_db_prep_value implementation as follows:

    def get_db_prep_value(self, value, connection, prepared=False):
        if not prepared:
            value = self.get_prep_value(value)
        return value

And that:

  • There are many Field children that override get_db_prep_value, without calling super(), but repeating the same lines explicitly:
    def get_db_prep_value(self, value, connection, prepared=False):
        if not prepared:
            value = self.get_prep_value(value)
        # do something else with value...
        return modified_value
  • IntegerField and BinaryField do call super() in get_db_prep_value :muscle:
  • A small subset of fields would not call get_prep_value in get_db_prep_value at all. These are DurationField, UUIDField.

Would it make sense to have those Field children preparing the value “by hand” calling super() instead? Shall we also call get_prep_value (one way or another) in the two fields that do not currently call it? (JSONField is already being fixed so I’m not counting it here).

2

I’m not opposed to making changes that make the usage of get_prep_value and get_db_prep_value more coherent in the core fields that Django. Given how often some of them are used for reference implementation it seems like a worthy change.

3

I opened a new ticket about the same subject before seeing your thread. There are some more fields listed there besides DurationField and UUIDField.