Add validation for duplicate lookups.

[fotoetienne]

Duplicate lookup fields creates ambiguity for the query planner

[netlify]

✅ Deploy Preview for composite-schemas ready!

Name	Link
🔨 Latest commit	88b6525
🔍 Latest deploy log	https://app.netlify.com/sites/composite-schemas/deploys/67b3965c8ecdbe00083776e0
😎 Deploy Preview	https://deploy-preview-170--composite-schemas.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[andimarek]

I think in order to allow for migration in a non breaking way we should allow deprecated fields. So it should be two or more non-deprecated lookup fields cause an error.

[fotoetienne]

I think in order to allow for migration in a non breaking way we should allow deprecated fields. So it should be two or more non-deprecated lookup fields cause an error.

That's a good idea. So the query planner would always choose the non-deprecated one, making it still unambiguous.

[michaelstaib]

I think in order to allow for migration in a non breaking way we should allow deprecated fields. So it should be two or more non-deprecated lookup fields cause an error.

we could discard of the deprecated lookup in the composition so that the gateway does not need to choose.

[martijnwalraven]

we could discard of the deprecated lookup in the composition so that the gateway does not need to choose.

But that would break existing clients that depend on the deprecated lookup field. So I don't think we want to remove them completely, just enforce that there's always exactly one non-deprecated lookup for composition and query planning to take advantage of.

[fotoetienne]

I think in order to allow for migration in a non breaking way we should allow deprecated fields. So it should be two or more non-deprecated lookup fields cause an error.

On further reflection, I believe migration could also be handled in a non-breaking way without allowing multiple @lookups.

before:

type Query {
    bookById(id: ID!): Book @lookup
}

after:

type Query {
    bookById(id: ID!): Book @deprecated
    bookByIdV2(id: ID!): Book @lookup
}

The subgraph would just move @lookup to the new field while leaving the old field present in the schema but without @lookup directive.

Perhaps for simplicity we can indeed restrict to a single @lookup without hindering migration.

[michaelstaib]

We need to move this to the validation section.