graphql-php icon indicating copy to clipboard operation
graphql-php copied to clipboard

QueryComplexityRule seems to attempt to parse input variables repeatedly

Open ingluisjimenez opened this issue 2 years ago • 2 comments

I am trying to enable the QueryComplexity rule and noticing that when the rule executes, it attempts to parse the values of the inputs multiple times for each Field definition.

The issue I am running into is that I have a custom scalar type that does some validation when parseValue() or parseLiteral() are called and log when it fails validation, and this validation process seems to be running (hundreds) multiple times, while I would expect that code to run at most once while the QueryComplexity is traversing through the schema. This ends up generating hundreds of error log lines when the custom scalar type fails validation, but also, it seems sub-optimal in that the rule attempts to extract/parse the same values from the input over and over.

Is this intentional behavior? Should my custom scalar type memoize the return value of parseValue() and prevent triggering the validation repeatedly?

ingluisjimenez avatar Oct 10 '23 16:10 ingluisjimenez

I guess the following call is repeated: https://github.com/webonyx/graphql-php/blob/aa7d23a598a1a5433a66f1144a9e5ba9c7ae0f7e/src/Validator/Rules/QueryComplexity.php#L123

Perhaps we can apply memoization in the implementation of the QueryComplexity rule?

spawnia avatar Oct 11 '23 07:10 spawnia

Thank you for following up @spawnia .

I am surprised this hasn't been brought up before as it seems something that would impact performance on a large graph and that's why I thought maybe I was doing something wrong.

It sounds like memoization would help to prevent calling Values::getVariableValues() multiple times, but buildFieldArguments() would still need to be called multiple times, as it is attempting to extract the arguments for a specific field/node, no?

ingluisjimenez avatar Oct 12 '23 17:10 ingluisjimenez