Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Meta] Document and improve support for Hasura & Dgraph schemas #272

Open
benjaminjkraft opened this issue May 6, 2023 · 8 comments
Open
Labels
documentation Improvements or additions to documentation enhancement New feature or request needs design Issues where we don't know exactly what we need yet; discuss on the issue before implementing
Milestone

Comments

@benjaminjkraft
Copy link
Collaborator

benjaminjkraft commented May 6, 2023

We seem to have a lot of users using genqlient to query a Hasura-generated schema, and they aren't having a great time of it; a lot of bugs and feature requests have come up over time. It would be good for us to take the time to do something a little more holistic, to see what support should look like. We can then document that, fix any gaps, etc. Similar issues may apply to Dgraph, and other tools with large generated schemas.

Specific feature requests can go in specific issues, this is more of a meta-issue to think about how we should support hasura, and to document the same.

The Problems

From reading all the issues mentioning Hasura, it seems like the main thing is the query syntax which has several interesting properties (discussed in #149 and #260):

  • it generates complex and often self-referential types
  • you usually only want to set a few fields of the very large input objects
  • it distinguishes implicit and explicit null (example)

These conflict badly with genqlient's defaults, and arguably with mapping the types to Go itself. (Some of these issues also apply to mutation inputs, but those are a bit closer to normal mutation inputs, and you probably use them in fewer places. Some of them also apply to other styles of input objects too, but hasura's are the most complex by far.)

There are also some other specific problems, which are more orthogonal but any documentation should mention too:

And some other things to document:

  • a list of good bindings for hasura types

From #206, the lunasec folks have a decently large example schema as an example. There's also a schema in #149. It could be interesting to have some tests against full hasura-generated schemas, or even integration tests against a hasura backend, to see how this looks.

What should we generate?

Some thinking out loud.

First of all, one theory -- already very possible -- is to say you just want to write your input object in the query. This is obviously less flexible, which in the common case is arguably good but doesn't scale well if you want to construct your queries with arbitrary Go logic, and doesn't handle certain cases like arbitrary-length lists of objects (although short of jsonb and arbitrary-length boolean it's not clear those really exist -- this is SQL after all). LunaSec is using this in several places (example).

Another theory is to just look at what (existing or plausible) options make hasura work better. I think use_struct_references is kinda close already; but that doesn't handle builtins. Global omitempty is a way out, but kinda a foot-gun for things like booleans. (LunaSec is doing that, via a fork.) Global omitempty-and-pointer is also an option -- hasura seems to now consider explicit null an error at least for all inputs if I'm understanding right? although on updates you might need it -- but that's annoying for builtins too. The problem is Go doesn't give us a lot to work with here for a "one-of" where you can't necessarily assume the go zero value is unused!

If we think a bit farther afield, I wonder about an API where we have separate constructors for each field you might want to set? It looks like you never per se need to set more than one (you can use an _and instead). So it would look something like

function QueryAuthorInputWhereNameEquals(string) QueryAuthorInput
function QueryAuthorInputAnd(...QueryAuthorInput) QueryAuthorInput

And then the particulars of how we implement QueryAuthorInput aren't as important. That's not the most composable -- the and syntax in particular is a bit ugly -- but maybe that's ok. You could always go back to the old way for special cases.

How should we configure?

Depending what we end up generating, we can look at how to configure genqlient to do so. One option is to figure out what a plugin would look like (#237). Another option is to have a special hasura: true config that does what you want, but probably better is to just document "here's a good set of defaults for hasura". We can see based on what it makes sense to generate.

Your feedback is welcome!

Hasura users, please let us know which of these ideas sound good to you, whether you have any other problems, etc.! And if your schema/queries are open source, posting those would help me see what you're trying to do.

@benjaminjkraft benjaminjkraft added documentation Improvements or additions to documentation enhancement New feature or request needs design Issues where we don't know exactly what we need yet; discuss on the issue before implementing labels May 6, 2023
@benjaminjkraft benjaminjkraft pinned this issue May 6, 2023
@benjaminjkraft benjaminjkraft changed the title Document and improve support for Hasura schemas [Meta] Document and improve support for Hasura schemas May 6, 2023
@kantum
Copy link

kantum commented May 8, 2023

Hello, I would love to see the documentation "here's a good set of defaults for hasura".

@nathanstitt
Copy link
Contributor

Hi @benjaminjkraft, thanks so much for the attention to this issue. I'm happy to answer any questions and pitch in on fine-tuning any tests you can think up.

I currently have a mid-size hasura schema with ~50 tables and a 35,720 line GQL schema dump 😭 I'll attach my current schema and the server's config & graphql file in hopes they're helpful.

I'm fortunate that most of my queries are done on the front-end and only sensitive admin-level ones are performed in go on the server. I currently have 12 mutations and 27 queries in my server-side code.

With the changes discussed in #149 and the use_struct_references: true config option coupled with numerous directives like: @genqlient(for: "account_insert_input.created", pointer: true, omitempty: true) it's been working well for me.

The largest issue I'm now facing is that I have timestamp columns mapped to time.Time but that causes mutations to insert 0 as a default value unless I add directives like the above. Then the trigger on the table sees the 0 and doesn't set the value to now() as it should. It's not quite risen to the level of digging in and making a PR for it, but it's on the TODO list.

Looking at my config, I'm also reminded that I fought with jsonb columns and finally just ended up using 'interface{}` and parsing internally. I only have one or two columns of jsonb, so it's not been a large issue for me.

schema-and-config.zip

@StevenACoffman
Copy link
Member

AFAICT, The Hasura GraphQL Engine serves a Relay-based schema for PostgreSQL tables which have a primary key defined.

As a result, I would think #78 for Relay specific issues is relevant.

Better support for Relay APIs, in general, would help not only Hasura, but Github API, and a number of other providers.

@dharijanto
Copy link

dharijanto commented May 9, 2023

Hi @benjaminjkraft , thanks a lot for paying attention to Hasura users, really appreciate it! :)

hasura seems to now consider explicit null an error at least for all inputs if I'm understanding right?

Not necessarily. Explicit null will not be an error for the following scenario. Instead, it will set the field to NULL validly. If we want it to fallback to "DEFAULT" value, however, we would need to omit the field altogether.
image

First of all, one theory -- already very possible -- is to say you just want to #260 (comment). This is obviously less flexible, which in the common case is arguably good but doesn't scale well

This is a very good observation! I think this is the answer for Hasura user. A client library I'm using in Flutter (https://github.com/heftapp/graphql_codegen) behaves this way and so far I haven't encountered issues using it with Hasura. Indeed it generates quite a lot of structures because it generates 1:1 structure for each of the GraphQL input, but I found it quite nice because it's easier to wrap my head around it.

@joshhayes-sheen-vt
Copy link

I'm not sure whether it's backed by Hasura or not, but I'm seeing similar problems when querying the graphql interface of https://www.datocms.com/, specifically the explicit vs implicit null, but also the complex/self-referential types which make application of per-type directives much harder

@kevinmichaelchen
Copy link
Contributor

Apologies if this has been reported elsewhere... I see a lot of value around polymorphic bulk mutations with conditionally present batch insert inputs... I wrote more about this problem here.

@partounian
Copy link

partounian commented Jun 1, 2023

I've also had to set use_struct_references: true and optional: pointer to make inputs to mutations for Braintree's / PayPal's GraphQL work reasonably. https://github.com/braintree/graphql-api/blob/master/schema.graphql

And now that I've gotten to queries, oh boy is this head scratching. If I'm understanding correctly, the issue might be similar to some of them that you're observing with Hasura?
Screen Shot 2023-06-01 at 12 48 37 PM

edit:

Ooh I might be simple minded or lack of docs, but GitHub Copilot seemed to get me the deeply nested query I needed right after making this comment.

@benjaminjkraft benjaminjkraft changed the title [Meta] Document and improve support for Hasura schemas [Meta] Document and improve support for Hasura & Dgraph schemas Jan 26, 2024
@benjaminjkraft benjaminjkraft added this to the v1.0 milestone Jan 26, 2024
@klondikedragon
Copy link

With my (fairly complex) Hasura generated GraphQL schema, it's pretty usable with global config options:

use_struct_references: true
optional: pointer

This is almost enough to be fully compatible with Hasura. For mutation inputs, Hasura treats different the presence of a field vs a field that has a null value. This requires sprinkling lots of omitempty: true in various places. However, this is dangerous because I actually only want omitempty added to fields that are considered optional in the GraphQL schema (where optional: pointer).

#308 looks perfect to provide a new "optional: pointer_omitempty" which will be absolutely perfect. I'll then have complete control over whether to provide an input field to Hasura based on whether the input struct field is nil or not, and not get messed up by zero values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request needs design Issues where we don't know exactly what we need yet; discuss on the issue before implementing
Projects
None yet
Development

No branches or pull requests

9 participants