strawberry-graphql / strawberry-django

Strawberry GraphQL Django extension
https://strawberry.rocks/docs/django
MIT License
409 stars 118 forks source link

Optimiser not working with custom prefetch query #389

Closed AlphaRishi1229 closed 3 months ago

AlphaRishi1229 commented 11 months ago

The strawberry django optimiser is not working as expected when we provide hints (like custom prefetch) for a field in a type. It results in n+1 queries when we do that

Describe the Bug

Suppose I have 2 models Model and ModelVariables, ModelVariables has a foreign key reference to Model. So, in django we can refer ModelVariables from Model using the related_name. Consider my models to be something like this:

class Model(model.Model):
    id = models.UUIDField(primary_key=True)
    name = models.CharField(max_length=255)

class ModelVariable(model.Model):
    id = models.UUIDField(primary_key=True)
    value = models.CharField(max_length=255)
    related_model = models.ForeignKey(
        "Model",
        on_delete=models.CASCADE,
        related_name="model_variables"
    )
    relational_field = models.ForeignKey(
        "RelatedField",
        on_delete=models.CASCADE,
        related_name="model_variables"
    )

class RelatedField(model.Model):
    id = models.UUIDField(primary_key=True)
    is_editable = models.BooleanField(default=False)

Since, ModelVariable has a foreign key to Model. I can get all the ModelVariable for a Model using -> Model.model_variables.all()

Now, for the above models I created strawberry django types in the following way

@strawberry_django.type(Model)
class ModelType(Node):
    id: NodeID

    @strawberry_django.field(prefetch_related=[
        Prefetch(
            "model_variables",
            queryset=ModelVariable.objects.filter(
                related_field__is_editable=True
            ),
            to_attr="editable_model_variables"
        )
    ])
    def editable_model_variables(self) -> list["ModelVariableType"]:
        return self.editable_model_variables

@strawberry_django.type(ModelVariable)
class ModelVariableType(Node):
    id: NodeID
    value: auto
    related_model: "ModelType"
    related_field: "RelatedFieldType"

@strawberry_django.type(RelatedField)
class RelatedFieldType(Node):
    id: NodeID
    is_editable: auto

Now, in my ModelType, I want editable_model_variables field to return all ModelVariables that are editable, so I added a prefetch_related for it and updated the query. Which works. Now, when I execute a query and mention related_field in the query. It causes n+1 queries. The obvious reason is that, in my prefetch related query I did not select_related the related_field.

AFAIK the optimiser takes these hints and not just override the optimised queries.

I also tried passing field_name="model_variables" to the @strawberry_django.field decorator for hinting but that didn't work too.

System Information

Upvote & Fund

Fund with Polar

diesieben07 commented 8 months ago

I think this should be possible fairly easily. If I understand this correctly, here an inspection of the field's prefetches should be added. If a Prefetch exists with to_attr being the field, then use that Prefetch to figure out the actual name of the model field. I will play around with this tomorrow if I can.

bellini666 commented 8 months ago

Hey @diesieben07 . Thanks for trying to help with this :)

Left me know if you need any help with that. Either ping me here or on our discord community

diesieben07 commented 8 months ago

Thank you for the heads up @bellini666. I got something working, it would be great to get some feedback about whether there are additional things that I have missed, as I am not deeply familiar with the optimizer's workings. I have created a pull request with more information about my changes: https://github.com/strawberry-graphql/strawberry-graphql-django/pull/473

bellini666 commented 7 months ago

Thanks @diesieben07 :)

Will take a look at it very soon!

Sanyambansal76 commented 6 months ago

@bellini666 and @diesieben07 any update on this?

diesieben07 commented 6 months ago

@Sanyambansal76 I will hopefully have time to look at the remaining work on my pull request on the weekend.