Fixed #29984 -- Support prefetch_related() with Queryset.iterator()

[RaphaelKimmig]

https://code.djangoproject.com/ticket/29984

[charettes]

There seems to be some cursor exhaustion issues for PostgreSQL. I haven't tested it but I wonder if psycopg2 allows us to have to have multiple cursors opened on the same connection at the same time. In this case we need way to keep a server side cursor opened while we iteratively open multiple client side cursors to perform the prefetches.

Did you investigate it a bit @RaphaelKimmig?

[RaphaelKimmig]

I've just looked into those failures and I think the issue was simply the tests using a batch size large enough to not keep a cursor open.

Given that this implementation now takes chunk_size results at a time, we'd only expect a cursor to remain open if there are more results than can be fetched in a single chunk.

I've updated the failing tests to use an appropriate chunk_size.

From what I understand having multiple cursors open at the same time should not be an issue, in fact there is a test for that (ServerSideCursorsPostgres.test_server_side_cursor_many_cursors).

[taylor-cedar]

@charettes What's the steps to getting this merged? This feature would be extremely useful.

[charettes]

@taylor-cedar AFAIK this is still a PoC because there isn't a clear consensus on how we should turn on the feature given backward compatibility concerns.

[charettes]

It'd be great if we could avoid this and all of the following if not self._prefetch_related_lookups

e.g.

# Avoid materialization of chunks of objects if no prefetching must take
# place.
if not self._prefetch_related_lookups:
    yield from self._iterable_class(self, chunked_fetch=use_chunked_fetch, chunk_size=chunk_size)
    return

That'll make sure the exact same behavior is preserved when no prefetching is involved and allow you to revert the test_server_side_cursors.py changes.

[RaphaelKimmig]

Yep, makes sense. I've reverted the test changes.

[charettes]

This conditional clause can be dropped once we perform an initial not self._prefetch_related_lookups as described above.

[charettes]

Why is this is required? I don't think there's a need to unref an empty list given clone is never returned?

[charettes]

Could use boolean not clone._result_cache check as well.

[charettes]

Could be reduced to yield from iter(clone._result_cache).

[felixxm]

Closing due to inactivity.