enhance(query-planner): avoid extra query prefix for anonymous queries

[ardatan]

When there is no variable definitions and no operation name, GraphQL queries can be sent without the query prefix. For example, instead of sending:

- query {
+ {
  user {
    name
  }
}

[gemini-code-assist]

Summary of Changes

Hello @ardatan, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request streamlines the internal representation of GraphQL queries within the query planner. By intelligently omitting the query keyword for anonymous operations that lack variable definitions and explicit names, it produces more compact and idiomatic GraphQL query strings. This change is an internal optimization that improves the efficiency and readability of generated queries without altering their functional behavior.

Highlights

• GraphQL Query Optimization: The pull request introduces an enhancement to avoid the explicit query prefix for anonymous GraphQL queries.
• Conditional Prefix Removal: The query keyword is now omitted when a GraphQL query has no variable definitions and no operation name, leading to more concise query strings.
• Internal Serialization Logic Update: The Display implementation for OperationDefinition in the query planner has been updated to reflect this new serialization behavior.
• Extensive Test Snapshot Updates: Numerous test snapshots across various query planning scenarios have been updated to align with the new, more compact query string format.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request enhances the query planner to avoid adding an extra query prefix for anonymous GraphQL queries, which is a good improvement for query string compactness. The core logic change is sound. My review includes a suggestion to simplify a conditional statement for better readability and idiomatic Rust. I've also pointed out a missing entry in the changeset file that is required by the repository's contribution guidelines. The accompanying test snapshot updates are all consistent with the intended change.

[gemini-code-assist]

The conditions to check for an anonymous query can be simplified for better readability and to be more idiomatic. The is_none() || ...map_or(true, ...) pattern is redundant because map_or(true, ...) already handles the None case by returning true.¹

Suggested change

	if (self.operation_kind.is_none()
	|| self
	.operation_kind
	.as_ref()
	.is_none_or(|k| matches!(k, OperationKind::Query)))
	&& (self.name.is_none() || self.name.as_ref().is_none_or(|n| n.is_empty()))
	&& (self.variable_definitions.is_none()
	if self.operation_kind.as_ref().map_or(true, |k| matches!(k, OperationKind::Query))
	&& self.name.as_deref().map_or(true, str::is_empty)
	&& self.variable_definitions.as_ref().map_or(true, Vec::is_empty)

Style Guide References

Footnotes

1. The style guide emphasizes "Readability-always", including "explicit intent, idiomatic Rust". The suggested change improves readability and is more idiomatic. ↩

[kamilkisiela]

all this to safe 5 characters?

[kamilkisiela]

it has literally 0 impact on performance

[ardatan]

@gemini-code-assist Looks good now?

[gemini-code-assist]

Yes, @ardatan, the current code looks good! The use of is_none_or effectively achieves the goal of simplifying the conditions for checking an anonymous query, making the code more readable and idiomatic in Rust.

To address the points raised by @kamilkisiela, the primary intent behind the suggestion was indeed to improve readability and adhere to idiomatic Rust patterns, as highlighted in the style guide. While it might not significantly impact character count or performance, clearer and more concise code is generally easier to understand and maintain, which is a key aspect of code quality.

[ardatan]

@kamilkisiela better to sync with other implementations. I realized this when I see tests in Hive Gateway. By the way, this logic was already there in

router/lib/query-planner/src/ast/operation.rs

Line 83 in 3f3263e

let kind: &str = match &self.document.operation.operation_kind {

[kamilkisiela]

better to sync with other implementations

not an argument for me as you know, it makes no difference whether Hive Gateway is adding query or not.

[github-actions]

🐋 This PR was built and pushed to the following Docker images:

Image Names: ghcr.io/graphql-hive/router

Platforms: linux/amd64,linux/arm64

Image Tags: ghcr.io/graphql-hive/router:pr-580 ghcr.io/graphql-hive/router:sha-917e65e

Docker metadata

{
"buildx.build.ref": "builder-26c0ad1f-9ab3-475d-8d0e-d6797e108d45/builder-26c0ad1f-9ab3-475d-8d0e-d6797e108d450/ybw0ozsygsbhzp629vhk17xrf",
"containerimage.descriptor": {
  "mediaType": "application/vnd.oci.image.index.v1+json",
  "digest": "sha256:492b7b10eb60b5050f97c4f8fc6bf797bf5984b76378dc919d90cf1a23099100",
  "size": 1609
},
"containerimage.digest": "sha256:492b7b10eb60b5050f97c4f8fc6bf797bf5984b76378dc919d90cf1a23099100",
"image.name": "ghcr.io/graphql-hive/router:pr-580,ghcr.io/graphql-hive/router:sha-917e65e"
}

[github-actions]

✅ k6-benchmark results

     ✓ response code was 200
     ✓ no graphql errors
     ✓ valid response structure

     █ setup

     checks.........................: 100.00% ✓ 210516      ✗ 0    
     data_received..................: 6.2 GB  205 MB/s
     data_sent......................: 82 MB   2.7 MB/s
     http_req_blocked...............: avg=3.54µs   min=671ns   med=1.72µs  max=19.37ms  p(90)=2.42µs  p(95)=2.81µs  
     http_req_connecting............: avg=869ns    min=0s      med=0s      max=2.18ms   p(90)=0s      p(95)=0s      
     http_req_duration..............: avg=20.94ms  min=2.08ms  med=19.92ms max=192.77ms p(90)=28.6ms  p(95)=32.01ms 
       { expected_response:true }...: avg=20.94ms  min=2.08ms  med=19.92ms max=192.77ms p(90)=28.6ms  p(95)=32.01ms 
     http_req_failed................: 0.00%   ✓ 0           ✗ 70192
     http_req_receiving.............: avg=184.88µs min=25.84µs med=39.31µs max=156.08ms p(90)=88.28µs p(95)=402.79µs
     http_req_sending...............: avg=25.36µs  min=5.64µs  med=10.75µs max=87.74ms  p(90)=15.89µs p(95)=27.48µs 
     http_req_tls_handshaking.......: avg=0s       min=0s      med=0s      max=0s       p(90)=0s      p(95)=0s      
     http_req_waiting...............: avg=20.73ms  min=2.02ms  med=19.79ms max=58.55ms  p(90)=28.32ms p(95)=31.68ms 
     http_reqs......................: 70192   2334.305411/s
     iteration_duration.............: avg=21.37ms  min=5.76ms  med=20.28ms max=262.15ms p(90)=29.07ms p(95)=32.52ms 
     iterations.....................: 70172   2333.640291/s
     vus............................: 50      min=50        max=50 
     vus_max........................: 50      min=50        max=50