Add a "strict mode" for defer()/only() to prevent queries on unfetched field access

[Adam Johnson]

In our project at YPlan we had some highly optimized queries using only() to restrict the fields being fetched.

Some time later the downstream code of these queries was changed to access extra fields not in the only() list. This triggered the DeferredAttribute implicit queries to go fill in those fields with single queries, a classic N+1 queries problem.

Whilst better testing on the queries would have revealed this, I can't help but feel that it's a somewhat surprising behaviour that only() (and by extension, defer()) can bite back this badly.

I'd like some way of making only() and defer() strict, and to raise an exception on deferred fields.

Currently we've implemented this with a patchy.patch to insert a raise into DeferredAttribute:

from django.db.models.query_utils import DeferredAttribute

patchy.patch(DeferredAttribute.__get__, """\
    @@ -20,6 +20,7 @@ def __get__(self, instance, owner):
             # might be able to reuse the already loaded value. Refs #18343.
             val = self._check_parent_chain(instance, name)
             if val is None:
    +            raise AttributeError("Deferred field '{}' fetched from database!".format(self.field_name))
                 instance.refresh_from_db(fields=[self.field_name])
                 val = getattr(instance, self.field_name)
             data[self.field_name] = val
""")

I can imagine this being a Django-wide setting, or maybe per-model so it wouldn't affect third party apps so badly.

[Tim Graham]

The API that comes to mind to me would be something like only(..., strict=True). Possible APIs should be discussed on the DevelopersMailingList before doing the implementation.

[Simon Charette]

FWIW Adrian proposed adding another keyword argument to only()/defer() that ​results in loading all fields.

Since we might want to add different kind of actions when a deferred field is accessed I suggest to use another name for the kwarg.

For example, we could issue a warning (warnings.warn) or log one (logging.warning) instead of raising an exception to prevent the code from crashing while exposing a way to monitor such instances.

[Anssi Kääriäinen]

Overriding Modelrefresh_from_db allows raising failure when deferred field is accessed - is this enough for you use case?

[Florian Apolloner]

Replying to charettes:

FWIW Adrian proposed adding another keyword argument to only()/defer() that ​results in loading all fields.

Loading all, or loading in groups as suggested by me in that thread and done in sqlalchemy (​http://docs.sqlalchemy.org/en/latest/orm/loading_columns.html#deferred-loading-with-multiple-entities) seems like a good idea.

For example, we could issue a warning (warnings.warn) or log one (logging.warning) instead of raising an exception to prevent the code from crashing while exposing a way to monitor such instances.

Since this feature would be optional for backwards compat anyways I do not see the need for logging.

[Simon Charette]

This has been implemented ​as a third-party application.

Not sure if this ticket should be closed as a duplicate of #22492 like #30874 was.

[Carlton Gibson]

Yes, it's the same ball-park certainly.

Since #22492 is Accepted, I wonder if bringing django-seal into core is on your agenda Simon?