Hi François, Thank you for a good write-up of the issue.
Having looked at this from each direction I can't see a way of squaring all the desires here. I see the possibility of an inconsistency between the __len__() and __iter__() calls. This would certainly be nice to avoid but I'm not sure it is the overriding concern here. I can easily see the memory issue being more significant than you're giving it credit for. The original issue (#23623) cited a ≈25% reduction in peak memory usage moving to the iterator. Choices are used in validation and not just template rendering. If not reset, the queryset is retained for the lifetime of the form instance, not just the ModelChoiceIterator iteration. With a large formset, with forms using ModelChoiceField on foreign keys with many thousands of records, the retained queryset would likely add up quickly. As such, I don't think we can just switch the default implementation. (I think we'd quickly see a lot of complaints.) I **do see** the appeal of re-using the queryset for a whole lot of cases. Many times I would trade a bit more memory for the lifetime of a form instance for one less query. (Although, just personally, I'm not sure how often `if field.choices` actually comes up, for me.) So, conclusions (with _I think we should…_ etc elided): * Close your PR as is. https://github.com/django/django/pull/9867 — We can't just change the behaviour. * Look at Tim's PoC PR. https://github.com/django/django/pull/8649 — Can we implement __len__() to not _fetch_all()? Can we implement the custom __bool__ to avoid `count()`? Can we do that without an extra DB query? * If we **can't** avoid the extra query then we have to decide whether it's an acceptable price? * wontfix: If it's not, the existing behaviour is _OK_: it's not incorrect for __len__() to _fetch_all(), and your previous fix makes sure we don't waste that effort if it has been done. * Fix: I'd guess the extra query probably is OK, especially with an optimised __bool__ just checking existence. It's the price of consistency here. * Can we then document, and maybe provide, an alternate ModelChoiceIterator that just re-uses the queryset? * Advantages of this are consistency, fewer DB hits etc. * Cost is the higher memory use. That's my take on the issue thus far. Kind Regards, Carlton -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at https://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/f8653583-4d19-4a06-9d68-f81639ea6937%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
