hasura / graphql-engine

Blazing fast, instant realtime GraphQL APIs on your DB with fine grained access control, also trigger webhooks on database events.
https://hasura.io
Apache License 2.0
31.17k stars 2.77k forks source link

Map UUID keys as GraphQL "ID" type #3578

Open nirvdrum opened 4 years ago

nirvdrum commented 4 years ago

Currently, Hasura creates a new scalar type called uuid for UUID keys. Because it's a non-standard type, using it with clients usually means jumping through some hoops to treat the value as a string. But, that's really all the standard ID type does anyway, as far as I know. I think if this type were remapped in the schema, it'd improve the ergonomics of working with a Hasura-based GraphQL server.

nirvdrum commented 4 years ago

Is there anything I can do to help usher this one along? It's a major pain point using Hasura with a ReasonML-based client.

coco98 commented 4 years ago

@nirvdrum (I think we were chatting on the community call a few minutes ago?)

Do you have any ideas on the best way of doing this?

Maybe an optional field in the metadata / in the console to mark any primary key column as an ID type? Considering an ID type is really just a name and a string serialization any postgres PK should work.

nirvdrum commented 4 years ago

I inquired about this on today's community call and it was asked that I provide more data on what I think the solution could look like.

According to the latest published version of the GraphQL Spec:

The ID scalar type represents a unique identifier, often used to refetch an object or as the key for a cache. The ID type is serialized in the same way as a String; however, it is not intended to be human‐readable. While it is often numeric, it should always serialize as a String.

Result Coercion

GraphQL is agnostic to ID format, and serializes to string to ensure consistency across many formats ID could represent, from small auto‐increment numbers, to large 128‐bit random numbers, to base64 encoded values, or string values of a format like GUID.

GraphQL servers should coerce as appropriate given the ID formats they expect. When coercion is not possible they must raise a field error.

Input Coercion

When expected as an input type, any string (such as "4") or integer (such as 4) input value should be coerced to ID as appropriate for the ID formats a given GraphQL server expects. Any other input value, including float input values (such as 4.0), must raise a query error indicating an incorrect type.

So, the spec not only permits UUIDs to be represented as the ID type, but it explicitly provides this use case as an example. As far as I know, the only sensible way to work with the custom scalar type uuid with Hasura is to treat it as a String coming in and going out. Thus, I think you can and should change the generated schema to return UUID PKs as the GraphQL ID type. It probably makes sense to do the same for FKs, but that may be trickier to work out. I don't know what Hasura does for int FKs today, but it should probably map the same way. For arbitrary UUID columns, the custom scalar likely still makes sense -- I think that boils down to interpretation of what the spec means when it says "represents a unique identifier". UUIDs are unique identifiers by definition, but they may not be unique in the backing database if stored in a non-unique column.

Since I'm advocating for a change in the generated schema, it could potentially break things for people. Maybe it'd make sense to have an option to ease migration. But, I truly believe mapping as ID by default is the best way to go.

coco98 commented 4 years ago

I think a Hasura metadata/console option that makes it easy to mark any UUID or Text column to an ID GraphQL type would work absolutely fine.

This should work well with the auto-generated resolvers and all the filter/sort options as well because Hasura can internally map the ID type back to UUID and text and all the operators will stay the same.

@0x777 @tirumaraiselvan Any thoughts? Any more input we need from @nirvdrum before we can turn this into a small RFC?

@nirvdrum Just for context for other folks on the team and anyone who comes across this issue, what kinds of things do you need to do on the reaonml client to make the UUID type work as a string type?

nirvdrum commented 4 years ago

@coco98 Thanks for the follow-up. An option in the console would be fine, too. I'd suggest maybe it should default to "on" in new tables if using the UUID PK "frequently used column" helper. I'd imagine this is something that'll come up as you consider Relay compatibility as well.

ReasonML is probably a niche use case, but I appreciate you considering it. Since ReasonML is statically typed, every GraphQL response needs to be decoded to some defined type. Since uuid is mapped as a custom scalar, graphql_ppx (a popular low-level library for working with GraphQL) doesn't know what to do with it, so it treats it as JSON object (Js.Json.t). If you're just recording the value and using it elsewhere for queries, there's no problem -- the key is already in the required type. But, anywhere where you might want to store the ID as a string (e.g., React component key), you have to do an awkward conversion to decode the JSON object:

let id = obj => {
  obj##id |> Js.Json.decodeString |> Belt.Option.getExn;
};

Likewise, if you have a string value somewhere that you want to use as a query variable, you need to encode it to a JSON object:

let makeId = id => {
  Js.Json.string(id);
};

It doesn't seem like much, but it's a mess of code needed for many interactions with every table. There are some ways this could be made nicer in ReasonML, but that's the issue broadly. I have a feature request open to provide global serializers for custom scalars to help ease this from the graphql_ppx side. But, I'd rather have my PKs mapped as GraphQL ID.

coco98 commented 4 years ago

Thanks for the context @nirvdrum. This is helpful! :)

nirvdrum commented 4 years ago

@coco98 No problem. Is there anything else I can do to help move this one along? It's made working with Hasura much uglier than I'd like. I was hoping this change was reasonably small enough to make its way into the 1.2.0 release.

tirumaraiselvan commented 4 years ago

Related: https://github.com/hasura/graphql-engine/issues/2881

TheMrZZ commented 4 years ago

Because of this issue, generating Typescript types for Relay (using relay-compiler) will assign the "unknown" type to UUIDs. It's a problematic issue that should not be there in the first place. Any update on this ?

philschonholzer commented 4 years ago

Creating a Remote Relationships with Sanity.io is not possible because of this.

jacdx commented 4 years ago

Is this issue on the roadmap? It has turned into a blocker for us, as we can't use Remote Schemas as-is, since they depend on IDs that are shared across systems (uuids in Hasura).

tirumaraiselvan commented 4 years ago

@jacdx Yes, this will be picked up very soon.

flobaader commented 3 years ago

@tirumaraiselvan Any updates on that issue? We're also unable to use Hasura with remote schemas in the meantime.

tirumaraiselvan commented 3 years ago

@flobaader We just released v1.3.4-beta1 which will allow joining String, Int or UUID types with ID type.

flobaader commented 3 years ago

@tirumaraiselvan Thank you very much for the quick response! I just tried the new beta and it works perfectly fine. You made my day! @philschonholzer You can now use Hasura with Sanity.io

jacdx commented 3 years ago

Woohoo!

On Wed, Nov 25, 2020 at 6:23 AM Florian Baader notifications@github.com wrote:

@tirumaraiselvan https://github.com/tirumaraiselvan Thank you very much for the quick response! I just tried the new beta and it works perfectly fine. You made my day! @philschonholzer https://github.com/philschonholzer You can now use Hasura with Sanity.io

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/hasura/graphql-engine/issues/3578#issuecomment-733737138, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARM2IWJHSGWGY34ZMPZZQDSRUHOLANCNFSM4J57W2YA .

carllippert commented 3 years ago

Just tried v1.3.4-beta.1 && beta.2 without success. Silently failing remote join on ID type in remote to UUID type in hasura. Extra bit of info is I'm trying to do it on a view.

Best way to diagnose?

carllippert commented 3 years ago

Ok tried 3 times and 3rd time worked. 🎉 Unsure why. v1.3.4-beta.1

nirvdrum commented 3 years ago

@coco98 @tirumaraiselvan Is there anything I can do to help move this along? This became a blocker for a project and I unfortunately haven't used Hasura much since. I could take a crack at a PR, but I don't want to sink time into something that's already being worked on. I also would hate to do it up and find out it doesn't match internal design goals.

nirvdrum commented 3 years ago

Is there any chance of getting this in for 2.0.0? If such a remapping poses a backwards-compatibility concern, doing it as part of the next major version could help.

chihshenghuang commented 3 years ago

Any update for this? I used v2.0.7 and I still can only use uuid type instead of the standard ID type

fishactual commented 3 years ago

This would be really useful!

1fexd commented 2 years ago

Hi, has there been any further development on this issue?

ArturHarutunyan commented 2 years ago

Hi there. I want to use Hasura with apollo gateway, but unfortunately, I get an error Unknown type uuid, also some with json and jsonb. and there is no way to declare custom scalars

nirvdrum commented 1 year ago

@coco98 @tirumaraiselvan @GavinRay97 I'm happy to help resolve this issue, but I'm not sure what it is I can do. The issue has been open for about three without any input from Hasura for the last two. I've just verified it's still a problem with 2.15.1. Is there any plan to resolve this or should we close the issue as "won't fix"?

kdawgwilk commented 1 year ago

Hi there. I want to use Hasura with apollo gateway, but unfortunately, I get an error Unknown type uuid, also some with json and jsonb.

and there is no way to declare custom scalars

This issue with Apollo gateway should be addressed in issue #9115

dameleney commented 1 year ago

This is on our roadmap. We do not have a timeline at present. Please monitor this issue for updates. A comprehensive RFC that covers all use cases and limitations of the feature will be posted on this issue.

@nirvdrum @kdawgwilk and others watching. Would the following satisfy your use cases? Provide the option to define within the scope of a Hasura project the default behavior for ID datatypes for requests and responses. The options would UUID (current), ID, STRING. Based on feedback on this ticket ID would be the default but we will have to consider backwards compatibility when making that decision.

nirvdrum commented 1 year ago

@dameleney I'm sorry for missing this notification. Being able to configure the type would solve the problem for me. I recognize backwards compatibility will be a concern and figured a configuration option at this point is unavoidable. I think you should change the default value is for new projects to be ID, however, since the current behavior is not what I suspect most people would expect. But, at the end of the day, so long as I can change it I'll be happy.

manasag commented 11 months ago

Thanks everyone for your comments and patience on this issue. We have been closely listening into all the feedback and requests from the community, and have been working on a re-imagined, re-architectured Hasura, that tackles many of these from ground up. We are pleased to announce that we are launching V3-Alpha of Hasura (Data Delivery Network) on our next Community call on Nov 30. The new metadata structure in V3 is designed to be highly flexible and declarative, to support the perfect GraphQL schema you would like to achieve, while leveraging the database specific features as much as possible. In V3, we provide the control to define the scalar mapping in the metadata, which would solve this issue. We would request to join this community call to learn more about Hasura V3. Post launch, we will update this issue with relevant details.