Interim performance optimization for large input arg types

adamni21 commented 2 months ago

Description

This PR aims to improve the performance for large input arg types. Eg. a filter input with many elements in the and/or field.

Discussed in discord: https://discord.com/channels/489127045289476126/498852330754801666/1261317211604123718

Only serves as an interim solution, while eval* is being removed/the filter plugin gets rewritten to be evaluated at execution time instead of planning time.

Performance impact

Vast improvement for the mentioned use case, >500x for a filter with 20 "or filters" on a filter type with ~250 fields. Maybe a slight performance decrease, for general usage, due to increased operation plan splitting.

Security impact

Unknown (likely none)

Checklist

[x] My code matches the project's code style and yarn lint:fix passes.
[x] I've added tests for the new feature, and yarn test passes.
[ ] I have detailed the new feature in the relevant documentation.
[ ] I have added this feature to 'Pending' in the RELEASE_NOTES.md file (if one exists).
[x] If this is a breaking change I've explained why.

I didn't add tests, but it should already be covered by grafast/dataplan-pg/__tests__/queries/conditions/complex-filter.test.graphql

Edit: Added an almost equivalent of the above test, which specifies the filters via variables, instead of inlining it into the query.

changeset-bot[bot] commented 2 months ago

🦋 Changeset detected

Latest commit: 4e102b1a1cd232e6f6703df0706415f01831dab2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 15 packages

| Name | Type | | -------------------------- | ----- | | @dataplan/pg | Patch | | grafast | Patch | | postgraphile | Patch | | graphile-build-pg | Patch | | graphile-utils | Patch | | pgl | Patch | | @localrepo/grafast-bench | Patch | | @dataplan/json | Patch | | @grafserv/persisted | Patch | | grafserv | Patch | | ruru-components | Patch | | @localrepo/grafast-website | Patch | | graphile-build | Patch | | graphile-export | Patch | | graphile | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

adamni21 commented 2 months ago

Excellent work; thanks for this! I've optimized it a bit, handled more edge cases, and added another test to check handling of partial variables (and undefined variables). I've also fixed some existing bugs I noticed at the same time which somehow have flown under the radar until now.

Lovely. Happy to help 😀

graphile / crystal