Loosening object identification GraphQL spec

[calebmer]

On a project I'm working on we are automatically generating a GraphQL server from a SQL server and many of our tables have an integer column named id. When serving this table through GraphQL we would like to make the id column queryable, however we also want our rows to implement a Node type in accordance to the Relay GraphQL spec. According to the spec an id field that is globally unique is required for compliance. This puts us in a binding position. Do we not allow columns to be named id? Do we rename id to rowId in GraphQL? These solutions aren't ideal as we also have many more foreign key columns like authorId which reference an id primary key.

The optimal solution would be if Relay allowed the id field to be renamed, either to _id or maybe gid (for global ID). Would it be possible to update the specification and Relay to allow for this? Is there a solution I'm not seeing?

[wincent]

I'd be reluctant to incur the internal complexity of making this kind of thing configurable, especially given that a work around is possible (exposing the underlying database's id field using another field name).

But leaking the underlying primary key sounds like a leaky abstraction to me anyway. I'd recommend that you only expose and use "global" IDs in your GraphQL schema (effectively base64encode(type + ':' + databaseID)). You shouldn't need to ever leak foreign keys like authorID in your schema because you'll typically be querying for them compositionally (ie. viewer { books { author } }) or via the node root field (ie. node(id: "someGlobalID")).

[calebmer]

The difficulty comes when introducing compound keys. There is no way (in my case) to intelligently compact the fields into a single field name and hiding these fields could also cause problems.

Another layer of complexity is added when these compound primary keys are also foreign keys.

There are some consumers that would greatly benefit from being able to access the data in the same form it is stored and other consumers (cough Relay cough) which would benefit from the Node format.

[wincent]

The difficulty comes when introducing compound keys. There is no way (in my case) to intelligently compact the fields into a single field name and hiding these fields could also cause problems.

Another layer of complexity is added when these compound primary keys are also foreign keys.

Can you provide a concrete example of these cases in a bit more detail, to ground the discussion? I want to make sure I fully understand the exact nature of the problem that you're facing.

[calebmer]

Take any join table that in a graph database would be an edge for instance. Say I have a table star and it has columns user_id and post_id (where id represents a serial integer id). The star table also has some metadata columns like created_at.

user_id references a user table's id column, post_id references a post table's id column and the post table also has a column author_id which references the user table's id column.

Another constraint (for me) is I can't custom design a GraphQL schema. It has to be generated from the detected relationships among the data.

[akre54]

Can't you have the client only be aware of global ids, and then using fromGlobalId on the server to map that to database objects? That's how we're doing it when the id-relations associations are essential.

relay/examples/todo/data/schema.js

Line 260 in da30726

const localTodoId = fromGlobalId(id).id;

[josephsavona]

Say I have a table star and it has columns user_id and post_id (where id represents a serial integer id). The star table also has some metadata columns like created_at. user_id references a user table's id column, post_id references a post table's id column and the post table also has a column author_id which references the user table's id column.

This would probably look something like:

type Star {
  id # use the helper to generate a global id from type + `id`
  user: User  # resolve() uses star.user_id to lookup the User
  post: Post  # resolve() uses star.post_id to lookup the Post
  created_at: Text
}
type Post {
  id # same here
  author: User
}
type User { ... }

Can you clarify where this would break down for your use case?

[calebmer]

@josephsavona it would break down at automatic generation for fields. I agree that schema would be the ideal, however I don't have the luxury to hide the primary key fields (I can't make assumptions about the data the schema is being generated from).

Would a solution like the following work with Relay, an id field with the following signature:

id(global: Bool)

Where global defaults to true? Then we could have a query like:

{
  me {
    id
    rowId: id(global: false)
  }
}

---

As a side note, I've opened sourced the project I'm working on so we can talk in more specifics if you want. Its here at PostGraphQL.

[ruslantalpa]

While making the name of the field (id) configurable might be complicated, choosing a better name for it (relay_id) so that it does not collide with just about every project schema naming convention would not be that complicated.
The problem is that users already using relay would need to rewrite their schema.
@wincent can you point out the places in the code where the "id" is hardcoded?

[ruslantalpa]

Any chance that this line is the only place that would need a change?
https://github.com/facebook/relay/blob/master/src/interface/RelayOSSNodeInterface.js#L38

[alloy]

Before I realised I was on the wrong repo, I filed a longer ticket here: graphql/graphql-relay-js#92

---

While making the name of the field (id) configurable might be complicated, choosing a better name for it (relay_id) so that it does not collide with just about every project schema naming convention would not be that complicated.

I agree with @ruslantalpa.

Using such a common and ambiguous field name is bound to lead to clashes for e.g. people that start with just GraphQL and later on want to add Relay.

What I think this thread is missing is that not all Relay clients can easily be updated when changing the semantics of an existing field that they’ve been using. In our case we have an iOS app and it’s very reasonable to assume that people will not update their app when we want them to.

I really do not like options, so my suggestion would also be to use a Relay specific ID field, if one exists, and otherwise fallback to id. Although, reading through the code, it seems like making the field configurable would be simpler (complexity-wise) than trying to reflect on the schema.

[alloy]

@wincent After code-spelunking a bit more, my thoughts are slightly different. Namely that Relay infers the primary key from the Node interface in the given GraphQL schema by looking for a field with type ID!.

With this traditional interface, Relay would use id as the primary key:

interface Node {
  id: ID!
}

But with this interface it would be relayID:

interface Node {
  relayID: ID!
}

---

I had wanted to provide a prototype patch, but after code-spelunking a full day I have to admit I’m slightly out of my depth with the code-base. My thoughts were:

• Add an optional primary key name parameter to the nodeDefinitions helper.
• babel-relay-plugin infers the primary key name by checking the Node interface in the given schema for a field that has the ID! type and attaches that field name as the inferredPrimaryKey metadata to the generated fields of types that have the Node interface.
• Relay uses the inferred primary key, but right now I’m not even sure what Relay uses that information for and it also doesn’t yet solve the hardcoded id value that @ruslantalpa linked to above.

[alloy]

I have been making some progress on this. With the following patches I’m able to define a custom Node ID field:

• A patched nodeDefinitions function to easily define your Node interface: alloy/graphql-relay-js@efb893d
• A patched babel-relay-plugin to inflect the ID field name from the Node interface in the schema: master...alloy:master

I still have some failing babel-relay-plugin tests to deal with after which I’ll start PRing.

[alloy]

Can somebody tell me if Relay should always include the id field in queries if that field exists on a type or if that just happened to be the case because Relay assumed that if an id field is present it is there because it implements the Node interface?

Here is where the current code always includes it, but I don’t see any tests that specifically describe that id should be included regardless of whether or not the type explicitly implements the Node interface.