Implement field merging

[bryanmylee]

What kind of change does this PR introduce?

Introduces field merging to properly combine multiple selections of the same object fields. This allows queries like:

query {
  blogPostCollection {
    edges {
      node {
        id
      }
    }
  }
  blogPostCollection {
    pageInfo {
      hasNextPage
    }
  }
}

... to resolve correctly to:

{
  "data": {
    "blogPostCollection": {
      "edges": [],
      "pageInfo": {
        "hasNextPage": false
      }
    }
  }
}

This PR also handles the case where two overlapping fields have different arguments, returning an error in the case.

query {
  blogPostCollection {
    edges {
      node {
        id
      }
    }
  }
  blogPostCollection(filter: {
    id: { eq: 1 }
  }) {
    pageInfo {
      hasNextPage
    }
  }
}

{
  "errors": [
    {
      "message": "Fields \"blogPostCollection\" conflict because they have differing arguments"
    }
  ]
}

What is the current behavior?

Object fields that appear more than once will override each other, resulting in only the last-seen field being returned.

{
  "data": {
    "blogPostCollection": {
      "pageInfo": {
        "hasNextPage": false
      }
    }
  }
}

This also affects #237 since overlapping fragment subqueries behave like overlapping fields.

What is missing?

Overlapping fields are merged correctly.

However, I still need to figure out how to determine the return type of a given field at the parser level so that I can return an error if two conflicting fields with different data types / shapes are merged with aliases.

There is also the issue of updating the positions and spans of a merged field. It doesn't affect the final output, but it may be useful in the future to define either a minimally containing span for all fields that make up a merged field, or store a vector of positions and spans.

[bryanmylee]

For additional context, I've added std::fmt::Debug traits to the T generic type that's passed around for easier debugging, but this can be removed if preferred.

Additionally, I've converted selections: Vec<&'b Field<'a, T>> into selections: Vec<Field<'a, T>> to more easily consume and mutate fields when merging. Specifically, using any sort of reference posed issues when I tried to consume the selections of a "from" field to be added to the selections of a mutable "to" field.

However, this does increase the heap allocation size and I'm probably not doing the best thing. Any suggestions to improve this would be appreciated.

take_mut is a zero-dependency crate that allows us to consume and replace a vector element in place with zero cost as far as I'm aware.

[olirice]

incredible stuff @bryanmylee!

I'll work through this tomorrow and follow up with any feedback

I'd like to get @imor 's take on this too since he's a much stronger rust developer. he's currently out sick so it might be a few days before he's able to get to it. just keeping you in the loop

[olirice]

I think this is confirming that the __Type of an and b are the same. As I understand, they need to be the exact same __Type in order to merge, correct?

If so, could we do something like make __Type hashable or equatable so they can be directly compared without this logic? The reason I prefer that kind of solution is because it would fail loudly if we introduce new __Types vs quietly not working with merging

[bryanmylee]

Yeah, this is a check to make sure both types have the same shape following the GQL specification linked in the discussion thread.

I did this explicitly because it gives me the ability to return specific errors for the different type conflicts, and keep the system close to spec.

I'm not sure if we should consider all __Type variants in the sameness check since it's not in spec, but an Eq trait would imply that the values are equal across across all variants.

However, I do agree it should be co-located closer to the __Type definition.

Another thing I just noticed was that I forgot to include type_name in the error messages so it's an unused variable.

[olirice]

I'm not sure if we should consider all __Type variants in the sameness check since it's not in spec, but an Eq trait would imply that the values are equal across across all variants.

That's interesting, I didn't quite follow. Could I hassle you to give an example that highlights the issue?

[bryanmylee]

My guess was if a __Type::Edge and __Type::Node are compared, they should not fail the shape sameness check yet, as the __Type::Node might have a compatible set of fields for a selection to be merged with the __Type::Edge.

Of course, I just realized this case would rarely be encountered since field selections have to have the same field name anyways, so I don't see where this issue might occur.

With that in mind, we can definitely just use a Eq check, unless we want to provide better error messages?