conform SQLCompiler.execute_sql() and results_iter() to Django's API

[WaVEV]

In order to be compatible with other SQL backends, execute_sql and results_iter must be changed.

The main idea is:
results_iter will always take values from execute_sql. It won't build queries.
The base queries from Django call both methods:
Something results iter after execute_sql:
We can see this on:
https://github.com/mongodb-forks/django/blob/mongodb-5.0.x/django/db/models/query.py#L91 and then in line 123.
Other times it uses execute_sql directly:
https://github.com/mongodb-forks/django/blob/mongodb-5.0.x/django/db/models/sql/query.py#L615.
In other cases, it calls the results_iter without results. There are plenty of examples in the test cases and also in the Django results_iter logic.

[timgraham]

I think we lost an optimization here where $project is omitted unless needed (i.e. there are annotations).

For example, the first query in basic.tests.ModelLookupTest.test_equal_lookup went from:

basic_article.aggregate([{'$match': {'$expr': {'$eq': ['$_id', ObjectId('668da533d38a1c8d3ee7708b')]}}}, {'$limit': 21}]);`

to

basic_article.aggregate([{'$match': {'$expr': {'$eq': ['$_id', ObjectId('668da57447faa4a8f994ebd1')]}}}, {'$project': {'_id': 1, 'headline': 1, 'pub_date': 1}}, {'$limit': 21}]);

We could fix it by making self.get_columns() if self.query.annotations else None to build_query() but there might be a better place to put it.

Adding a test to the Django fork for this at some point would probably be a good idea.

[WaVEV]

Yes, I added this new layer thinking that is not a big deal. If I change the code a bit, the old behavior is preserved. So, I will change this code in order to keep the previous behavior.

[timgraham]

I'm not sure how much it matters performance-wise, but particularly when debugging queries, I think it's cleaner not to include $project if it's not needed, as long as the solution for doing so doesn't get super complicated.

[WaVEV]

it's not. As you mention above, we need to pass a None to build query if not self.query.annotation. Running the test, will update soon.

[timgraham]

chop blank line

[timgraham]

I see you added the get_default() bit back in a separate commit. Were there any test failures before that? The code came from the mongo engine fork and I've been skeptical of it. It seems it could be related to missing columns due to schema changes, but I'm not sure it's the behavior we want. Just wondering if you have any analysis to add.

[WaVEV]

Yes, there was. A JSONField test, don't remember the name (not_provided wasn't a json). But basically I copied the logic from Django. It is not that deep like types changes or so.
Currently we are flagging those fields with NOT_PROVIDED, then we check if it not provided we assign the default, other case we apply converters. you can see the code here:
https://github.com/WaVEV/django-mongodb/blob/main/django_mongodb/compiler.py#L94
Now a default value is assigned and then don't care about the converters. The only thing to point out is: the code is running converters over default values. But Django does the same if I am not mistaken.

[timgraham]

Well done!