Relax SameResponseShape algorithm to be compatible with covariant fields

[IvanGoncharov]

At the moment covariance rules for interfaces are incompatible with
validation rule for overlapping fragments.
For example, if we have schema like this:

interface AddressInterface {
  country_code: String
}

type Port implements AddressInterface {
  country_code: String!
}

type Warehouse implements AddressInterface {
  country_code: String
}

type Query {
  addressInterface: AddressInterface
}

This query is valid:

query {
  addressInterface {
    country_code
  }
}

But if you expand interface field into inline fragments will cause a
validation error:

query {
  addressInterface {
    ... on Port {
      country_code
    }
    ... on Warehouse {
      country_code
    }
  }
}

This PR fixes this issue.

[leebyron]

Thanks for the detailed example, I'd love to see this fixed, looking forward to discussing this!

[leebyron]

is this sufficient? what about more complex covariance scenarios?

[leebyron]

I think this is what we need to compare against. Two valid branches into https://spec.graphql.org/draft/#IsValidImplementationFieldType()

[andimarek]

I think that is correct: we would have to consider the same logic as linked above.

Edit: Actually as showed in another comment covariant interface situations are already allowed. At least in some situations at least.( I am not sure at the moment what else is missing.)

But I want to call out that this change doesn't take any hierarchy into account. So this would not only relax the overlapping situation for covariant field but would allow to be merged which are not covariant related in any way.

[leebyron]

(Notes from in person discussion in WG)

My counter-argument is that this is actually working as intended, even though it's unintuitive.

While there have been some other past challenges to the purpose of SameResponseShape() so far we've agreed that it is a valuable constraint.

The example here is an edge case for sure, but still seems to be introducing two fields that not going to return the "same" type, even though they are covariant to the interface they both implement, and so the constraint should still apply.

---

Three options:

1. Agree this is unintuitive but works as advertised and make no change
2. Determine "SameResponseShape" is too strict and loosen the restriction specifically to null (essentially what this PR does)
3. Add a specific case to "SameResponseShape" that allows interface implemented fields to be considered "Same" even though they are not the "same" type (but covariant) - targeting specifically this example issue.

My sense is that 3 is not really viable in that it has a similar impact to 2 but more complexity. I loosely think 1 is the right path given our constraints are working as intended, but if we do decide to pursue 2 (what's implemented here) then we should zoom out and better understand the impact of that decision

[leebyron]

An interesting RFC to compare this to is the one introducing ! forced non-null fields. We might rely on a similar rule (or this one) to determine where we allow merging such fields and where we require aliases

[IvanGoncharov]

Moving to Stage1 per WG discussion https://github.com/graphql/graphql-wg/blame/main/notes/2021-09-02.md#L200-L201

[andimarek]

I want to document for completeness sense that not all covariant fields are restricted in this way. You can actually query two different covariant fields IF the covariant type is not a Scalar/Enum.
For example:

 type Query {
    pets: [Pet]
  }
  interface Pet {
    name: String
    owner: PetOwner
  }
  type Dog implements Pet {
    name: String
    owner: DogOwner
  }
  type Cat implements Pet {
    name: String
    owner: CatOwner
  }
  interface PetOwner {
    name: String
  }
  type DogOwner implements PetOwner {
    name: String
  }
  type CatOwner implements PetOwner {
    name: String
  }

Here owner is covariant for Cat and Dog.
This query is valid:

{
  pets {
    ... on Dog {
      name
      owner {
        name
      }
    }
    ... on Cat {
      name
      owner {
        name
      }
    }
  }
}

Just to be clear: the reason is actually not that covariant is specifically handled, but the specific composed type returned doesn't matter when fields overlap.

[IvanGoncharov]

Closing this proposal in favor of #895
Since it allows to implement of this transformation if add ? to every field.