apollographql / router

A configurable, high-performance routing runtime for Apollo Federation 🚀
https://www.apollographql.com/docs/router/
Other
816 stars 271 forks source link

Aliasing a field to a @key field name yields Rust query planner error #6222

Open carldunham opened 3 weeks ago

carldunham commented 3 weeks ago

Describe the bug

Demonstrated in https://github.com/carldunham/apollo-router-6222.

Given the attached schema, this query

query Broken {
  animals {
    id: name
    name
    hello
  }
}

will succeed, but the logs show

2024-11-02T20:28:04.778921Z WARN  Rust query planner error in query `Broken`: An internal error has occurred, please report this bug to Apollo.

Details: Cannot merge field selection for field "Animal.name" into a field selection for field "Animal.id"

Similar queries will provide correct results and no issues logged:

query Works {
  animals {
    id
    name
    hello
  }
}

query WorksToo {
  animals {
    ident: id
    name
    hello
  }
}

To Reproduce

Steps to reproduce the behavior:

  1. Clone the provided repo
  2. Link, download, or copy whatever version of rover and router you want to test with
  3. Run make supergraph run
  4. Execute the provided query in the sandbox
  5. See error in Router log
  6. (if you want to see results, run the subgraph services like (cd a && make run)

Expected behavior

No issues logged.

Output

See above.

Desktop (please complete the following information):

Additional context

Tested with various recent versions, up to

carldunham commented 3 weeks ago

Note that setting

experimental_query_planner_mode: new

results in a query failure, naturally:

{
  "errors": [
    {
      "message": "value retrieval failed: Federation error: An internal error has occurred, please report this bug to Apollo.\n\nDetails: Cannot merge field selection for field \"Animal.name\" into a field selection for field \"Animal.id\"",
      "extensions": {
        "code": "INTERNAL_SERVER_ERROR"
      }
    }
  ]
}
goto-bus-stop commented 3 weeks ago

Thanks for the report. This is a known issue, I don't think we have a specific time frame in mind for addressing it (potentially after we remove the JS implementation of query planner from the router)

carldunham commented 3 weeks ago

potentially after we remove the JS implementation

But this would be a regression in Rust, no?

goto-bus-stop commented 3 weeks ago

I believe JS silently does the wrong thing in this case 🙈 (overwriting the aliased selection)

carldunham commented 3 weeks ago

Indeed it does!