strawberry-django icon indicating copy to clipboard operation
strawberry-django copied to clipboard

Optimiser not working with custom prefetch query

Open AlphaRishi1229 opened this issue 10 months ago • 6 comments

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

  • Operating system: Ubuntu 22.04
  • Strawberry version (if applicable):
    • strawberry-graphql-django==0.20.0
    • strawberry-graphql==0.209.5

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar

AlphaRishi1229 avatar Oct 11 '23 10:10 AlphaRishi1229

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.

diesieben07 avatar Jan 29 '24 23:01 diesieben07

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

bellini666 avatar Feb 03 '24 16:02 bellini666

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

diesieben07 avatar Feb 05 '24 21:02 diesieben07

Thanks @diesieben07 :)

Will take a look at it very soon!

bellini666 avatar Feb 07 '24 22:02 bellini666

@bellini666 and @diesieben07 any update on this?

Sanyambansal76 avatar Mar 14 '24 13:03 Sanyambansal76

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

diesieben07 avatar Mar 14 '24 14:03 diesieben07