django-seal icon indicating copy to clipboard operation
django-seal copied to clipboard

Don't raise unfetched many-to-many warnings when fetching a single item.

Open charettes opened this issue 5 years ago • 2 comments

There's no point in raising a violation for unfetched many-to-many relationship when explicitly fetching a single object .get(), .first(), [index] since it would result in the same number of queries.

charettes avatar Jun 17 '20 20:06 charettes

@charettes thanks again for this great project.

Would this issue also encompass a situation where I have a single object instance and want to get a related table, i.e.,

my_object = MyObject.objects.get(pk=some_pk)
my_related_object = my_object.sealed_related_object.my_related_object  # this currently raises using django-seal

I'm trying to use django-seal only to reduce major N+1 queries on a queryset, and I'm not really concerned with any type of optimization on the above code. Is this what #48 says it is starting to resolve? If yes is there anything I could do to help push the PR forward?

YPCrumble avatar Oct 16 '23 18:10 YPCrumble

@YPCrumble 👋 hello there!

The idea behind #48 was that in the case of many-to-many relationships Django currently doesn't allow you to fetch them at the same time as the object you are retrieving as prefech_related incurs an additional query for each relationship.

This means that in the case of retrieval of a many-to-many relationship of an object retrieved via .get() the N+1 query problem is bound as N=1.

In the case of single-valued relationships though you have select_related which can be used to retrieve all relationship in a single query (which also happens to ensure integrity of your relationships) so the problem is a bit different. If we are to add a mode to not warn about unsealed many-to-many accesses from a single retrieved object then it shouldn't be too hard to add one for single-valued as well. Maybe that #68 could help with that to a certain extent?

If yes is there anything I could do to help push the PR forward?

I think that gathering context about how other ORMs do it and what are the possible mode we'd want to support could help? Currently this tool has two modes; seal or unsealed. What kind of use cases would a new API need to support?

I can see three mutually exclusive now (better names welcome)

  1. all; all unsealed attributes access are surfaced
  2. optimizable; what this issue is about
  3. variable; all objects that could have a variable amount of queries (not-constant) surface warnings

charettes avatar Oct 16 '23 18:10 charettes