typelevel / grackle

Grackle: Functional GraphQL for the Typelevel stack
http://typelevel.org/grackle/
Apache License 2.0
176 stars 25 forks source link

Fully implement field selection merging and collection rules #550

Closed milessabin closed 8 months ago

milessabin commented 8 months ago

See sections 5.3.2 and 6.3.2 of the GraphQL Specification.

Mergeability validation is done very early in compilation, immediately after variable and fragment use is validated.

As much field merging is done during compilation as possible. In some cases this will result in simpler executable queries than were previously generated which might slightly improve query execution time.

Field merging which depends on the runtime type of values has to be deferred to execution time, and potentially involves deep merging of ProtoJson result values. This could be expensive, and considerable effort has been taken to avoid it where possible and reduce its impact where not.

In almost all cases where this PR changes behaviour the previous behaviour was incorrect. The only changes in previously correct results are potential object field reorderings in Json results.

A handful of existing tests were violating the mergeability rules and have been corrected.

Fixes #22

codecov-commenter commented 8 months ago

Codecov Report

Attention: 23 lines in your changes are missing coverage. Please review.

Comparison is base (63a6f10) 73.22% compared to head (f2d2da2) 73.80%.

Files Patch % Lines
modules/core/src/main/scala/queryinterpreter.scala 73.33% 16 Missing :warning:
modules/core/src/main/scala/compiler.scala 92.30% 7 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #550 +/- ## ========================================== + Coverage 73.22% 73.80% +0.57% ========================================== Files 32 32 Lines 4456 4607 +151 Branches 997 1060 +63 ========================================== + Hits 3263 3400 +137 - Misses 1193 1207 +14 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.