Skip to content

Query planner fails with message: **non common supertype** #2466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
andrew-kolesnikov opened this issue Mar 14, 2023 · 4 comments · Fixed by #2467
Closed

Query planner fails with message: **non common supertype** #2466

andrew-kolesnikov opened this issue Mar 14, 2023 · 4 comments · Fixed by #2467
Assignees
@pcmanus

Comments

@andrew-kolesnikov
Copy link

andrew-kolesnikov commented Mar 14, 2023 •
edited
Loading

Rust query planner fails with a cryptic error non common supertype, while JavaScript server accepts this same query with same schema just fine.

To Reproduce

  1. Setup Schema
type Query {
    orderById(orderId: UUID!): FullOrRedactedOrder
}
union FullOrRedactedOrder = CustomerOrder | CustomerRedactedOrder

type CustomerOrder {
   //...
    delivery: Delivery
}

type CustomerRedactedOrder {
    //...
    delivery: RedactedDelivery
}

type Delivery {
    deliveryId: UUID!
    //...
    status: DeliveryStatus!
  }
  
type RedactedDelivery {
    deliveryId: UUID!
    //...
    status: DeliveryStatus!
  }

enum DeliveryStatus {
    COMPLETED
}
  1. Submit request
{
    "query":"query OrderById($orderId: UUID!) {
                orderById(orderId: $orderId){
                    ... on CustomerOrder {
                        delivery{
                            status
                        }
                    } 
                    ... on CustomerRedactedOrder {
                        delivery{
                            status
                        }
                    }
                }
            }
    ","variables": {
        "orderId": "d30593ce-3dd2-4b04-b58c-c3fXXXXXXXXa"
    }
}
  1. See error
Object {\\\"code\\\": String(\\\"QUERY_PLANNING_FAILED\\\"), \\\"exception\\\": 
Object {\\\"stacktrace\\\": 
Array [String(\\\"Error: Cannot add { ... on RedactedDelivery { __typename deliveryId } } of parent type RedactedDelivery
 to { ... on Delivery { __typename deliveryId } } of parent type Delivery: non common supertype\\\")]}}\", line: 0, column: 0)``.
@Geal
Copy link
Contributor

Geal commented Mar 15, 2023

hi! could you tell me which versions of the router and gateway you tested this with?

@Geal Geal transferred this issue from apollographql/router Mar 15, 2023
@pcmanus
Copy link
Contributor

pcmanus commented Mar 15, 2023

Had a look, and I can reproduce in the query planner directly so almost certain it's a query planner bug, not a router one (hence the transfer). But I also think it's a regression from a newer federation version, which might be one it might work with some versions of the gateway. I'll have a closer look.

pcmanus pushed a commit to pcmanus/federation that referenced this issue Mar 15, 2023
Fix assertion errors when querying types with no common supertypes at…
2093d61 
… the same path

Fetches input selections used to be based on the queried subgraph
schema. However, those selections truly applies to the in-memory data
maintained by the gateway/router, so the supergraph. This used to
not matter (all input selection did correctly applied to the subgraph
schema), but `@interfaceObject` made it so that some input selections
could be invalid for the subgraph. Typically, a fetch that queries a
subgraph with an `@interfaceObject` may look like:
```
Fetch(service: 'A') {
  {
    ... on Book {
      id
    }
  } => {
    ... on Product {
      <some fields>
    }
  }
}
```
where `Book` is a concrete implementation of `Product` but unknown to
subgraph `A`.

However, basing the input selections on the supergraph schema means
having the parent type, in the supergraph, for those selections, and
because some fetches can query multiple different entities, this is
not trivial.

The initial code for `@interfaceObject` make the (incorrect) assumption
that, when a fetch does query multiple types, then we could always find
a common supertype for all those types, and so was using that type as
parent type.

Unfortunately, this isn't always true (see test in this PR, which uses
a union type with types having a field with the same name but different
(and unrelated) types).

So in pratice, fetch inputs cannot always have a common parent type.
This patch thus changes the fetch inputs to be a map of type to
selection sets instead of a single selection set.

Note that it does mean that the `requires` field of a `FetchGroup` is
not a true valid "single" selection for the supergraph (or any subgraph
for that matter), but it is more of a "fun fact" since the execution
never relied on this anyway.

Fixes apollographql#2466
@pcmanus pcmanus mentioned this issue Mar 15, 2023
Merged
@pcmanus
Copy link
Contributor

pcmanus commented Mar 15, 2023

Confirming that this a regression of 2.3+. I made some incorrect assumptions when doing some changes for @interfaceObject, leading to this assertion error being triggered. I've pushed a fix at #2467 to fix.

@pcmanus pcmanus self-assigned this Mar 15, 2023
@andrew-kolesnikov
Copy link
Author

andrew-kolesnikov commented Mar 15, 2023 •
edited
Loading

Rust Router tested (bug found): 1.10.3
nodejs version tested: (no bug):

        "@apollo/gateway": "^2.1.4",
        "@apollo/subgraph": "^2.1.4",
        "apollo-server-core": "^3.11.1",
        "apollo-server-errors": "^3.3.1",
        "apollo-server-express": "^3.11.1",

pcmanus pushed a commit that referenced this issue Mar 15, 2023
Fix assertion errors when querying types with no common supertypes at…
09382e7 
… the same path (#2467)

Fetches input selections used to be based on the queried subgraph
schema. However, those selections truly applies to the in-memory data
maintained by the gateway/router, so the supergraph. This used to
not matter (all input selection did correctly applied to the subgraph
schema), but `@interfaceObject` made it so that some input selections
could be invalid for the subgraph. Typically, a fetch that queries a
subgraph with an `@interfaceObject` may look like:
```
Fetch(service: 'A') {
  {
    ... on Book {
      id
    }
  } => {
    ... on Product {
      <some fields>
    }
  }
}
```
where `Book` is a concrete implementation of `Product` but unknown to
subgraph `A`.

However, basing the input selections on the supergraph schema means
having the parent type, in the supergraph, for those selections, and
because some fetches can query multiple different entities, this is
not trivial.

The initial code for `@interfaceObject` make the (incorrect) assumption
that, when a fetch does query multiple types, then we could always find
a common supertype for all those types, and so was using that type as
parent type.

Unfortunately, this isn't always true (see test in this PR, which uses
a union type with types having a field with the same name but different
(and unrelated) types).

So in pratice, fetch inputs cannot always have a common parent type.
This patch thus changes the fetch inputs to be a map of type to
selection sets instead of a single selection set.

Note that it does mean that the `requires` field of a `FetchGroup` is
not a true valid "single" selection for the supergraph (or any subgraph
for that matter), but it is more of a "fun fact" since the execution
never relied on this anyway.

Fixes #2466
pcmanus pushed a commit that referenced this issue May 16, 2023
Fix assertion errors when querying types with no common supertypes at…
ce0459a 
… the same path (#2467)

Fetches input selections used to be based on the queried subgraph
schema. However, those selections truly applies to the in-memory data
maintained by the gateway/router, so the supergraph. This used to
not matter (all input selection did correctly applied to the subgraph
schema), but `@interfaceObject` made it so that some input selections
could be invalid for the subgraph. Typically, a fetch that queries a
subgraph with an `@interfaceObject` may look like:
```
Fetch(service: 'A') {
  {
    ... on Book {
      id
    }
  } => {
    ... on Product {
      <some fields>
    }
  }
}
```
where `Book` is a concrete implementation of `Product` but unknown to
subgraph `A`.

However, basing the input selections on the supergraph schema means
having the parent type, in the supergraph, for those selections, and
because some fetches can query multiple different entities, this is
not trivial.

The initial code for `@interfaceObject` make the (incorrect) assumption
that, when a fetch does query multiple types, then we could always find
a common supertype for all those types, and so was using that type as
parent type.

Unfortunately, this isn't always true (see test in this PR, which uses
a union type with types having a field with the same name but different
(and unrelated) types).

So in pratice, fetch inputs cannot always have a common parent type.
This patch thus changes the fetch inputs to be a map of type to
selection sets instead of a single selection set.

Note that it does mean that the `requires` field of a `FetchGroup` is
not a true valid "single" selection for the supergraph (or any subgraph
for that matter), but it is more of a "fun fact" since the execution
never relied on this anyway.

Fixes #2466
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pcmanus pcmanus

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix assertion errors when querying types with no common supertypes at… pcmanus/federation
3 participants
@Geal @pcmanus @andrew-kolesnikov