Serializer makes unnecessary joins

[Juan Alvarez]

When handling M2M fields, the serializer would make unnecessary joins, as it has an empty select_related. Its default behavior is to select all related fields. This can even trigger a recursive addition of related fields, that end up in an OperationalError because of too many joins.

The regression seems to have been introduced in https://code.djangoproject.com/ticket/34620

[Natalia Bidart]

Hello Juan, thanks for your report.

Do you think you could provide a reproducer? A small django test project, or a set of models exercising the issue, or a failing test case. That way we can triage the issue more accurately.

Natalia.

[Juan Alvarez]

Replying to Natalia Bidart:

Hello Juan, thanks for your report.

Do you think you could provide a reproducer? A small django test project, or a set of models exercising the issue, or a failing test case. That way we can triage the issue more accurately.

Natalia.

Natalia,

For reference, I did submit a potential fix at ​https://github.com/django/django/pull/17164

Actually, the existing test did _kind_ of covered the regression. The issue was that since the model had null=True, the empty select_related() did not use the field, because of:

def select_related_descend(field, restricted, requested, select_mask, reverse=False):
    # [...]
    if not restricted and field.null:
        return False

So this change:

tests/serializers/models/base.py

@@ -60 +60 @@
 
 class Topic(models.Model):
     name = models.CharField(max_length=255)
-    category = models.ForeignKey(Category, models.CASCADE, null=True)
+    category = models.ForeignKey(Category, models.CASCADE)
     objects = TopicManager()

Makes this test fail:

    def test_serialize_only_pk(self):
        with self.assertNumQueries(7) as ctx:
            serializers.serialize(
                self.serializer_name,
                Article.objects.all(),
                use_natural_foreign_keys=False,
            )

        categories_sql = ctx[1]["sql"]
        self.assertNotIn(connection.ops.quote_name("meta_data_id"), categories_sql)
        meta_data_sql = ctx[2]["sql"]
        self.assertNotIn(connection.ops.quote_name("kind"), meta_data_sql)
        topics_data_sql = ctx[3]["sql"]
        self.assertNotIn(connection.ops.quote_name("category_id"), topics_data_sql)

because there is a join to category_id when Topic.category is not nullable.

If you consider this not sufficient, I can try to come up with a more complete example. But this issue is breaking our application, and the proposed fix does fix it.

[Mariusz Felisiak]

Good catch! Thanks for the report and patch.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 517d3bb4:

Fixed #34779 -- Avoided unnecessary selection of non-nullable m2m fields without natural keys during serialization.

By using select_related(None) instead of select_related(), the
unnecessary joins are completely avoided. Note that the current tests
already covers the change, when the field is not null=True.

Regression in f9936deed1ff13b20e18bd9ca2b0750b52706b6c.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 46b2b08e:

[4.2.x] Fixed #34779 -- Avoided unnecessary selection of non-nullable m2m fields without natural keys during serialization.

By using select_related(None) instead of select_related(), the
unnecessary joins are completely avoided. Note that the current tests
already covers the change, when the field is not null=True.

Regression in f9936deed1ff13b20e18bd9ca2b0750b52706b6c.

Backport of 517d3bb4dd17e9c51690c98d747b86a0ed8b2fbf from main