Prisma1: Go client: use int instead of int32

Created on 21 Feb 2019  ·  15Comments  ·  Source: prisma/prisma1

Describe the bug

Prisma generates int32 types, which is cumbersome to work with:

func (r *queryResolver) Feed(ctx context.Context, pagination *graphql.Pagination) ([]prisma.Post, error) {
    published := true
    s := int32(*pagination.Skip)
    return r.Prisma.Posts(&prisma.PostsParams{
        Skip: &s,
        Where: &prisma.PostWhereInput{Published: &published},
    }).Exec(ctx)
}

type Pagination struct {
    Skip  *int `json:"skip"`
    First *int `json:"first"`
}

Origin:

https://github.com/prisma/prisma/blob/ff6ae93a7b101827ef0ea598e105758c4899ac55/cli/packages/prisma-client-lib/src/codegen/generators/go-client.ts#L52

Why is int32 used and not int? Am I missing something? I think int should be used as it's recommended to use this over more specific types.

If approved, I'll gladly send a PR.

kindiscussion arecliengo

All 15 comments

@dominikh would you mind to comment on this one? 🙏

As @pantharshit00 mentioned on Slack, this idea probably comes from the GraphQL spec which describes the Int type as a 32-bit integer. I use gqlgen for the GraphQL resolvers and gqlgen uses the int type – maybe gqlgen should generate int32s instead.

However, I'd often still have to use var a int32 = 5 instead of a := 5

I do not have a good answer to this. Yes, it is preferred to use int in Go when possible, since it's the default type for integer literals, and most functions return ints. However, its size is platform specific (32 or 64 bits) and the Go types become less specific than the GraphQL schema if we use int.

Furthermore, we would have to discuss how we handle overflow. Would each method check that the user-provided int fits into 32 bits and returns an error otherwise? Or would it unconditionally convert to int32 for the backend and risk wrap-around?

@vektah would you mind providing your perspective here?

I think int is more idiomatic, int32 isn't adding any more validation, it's just overflowing silently as Dom points out. Pushing the user to check means this validation will not get done. It should be in the library as close to the raw input as possible.

Is there an escape hatch if you want larger numbers? Eg 64, or even bigint? I guess it's just string?

I use gqlgen for the GraphQL resolvers and gqlgen uses the int type – maybe gqlgen should generate int32s instead.

You can do this yourself if you like, just copy out the default implementation, change it to int32 and override it in your config.

Seems like we should then go ahead and change from int32 to int. Any objections @dominikh?

@steebchen would be great to raise a PR for this 🙏

No objections from me if you're happy with it.

Okay. ~What about float64? It seems that gqlgen maps Int to int and Float to float64 (@vektah why though?)~.

is the issue still open? i am facing the issue even in latest prisma (1.28.1)

@krishnakprakash Yes, the change is suggested in #4121 but is not merged yet

I am still facing this issue, can someone please help me out

cc @matthewmueller

I had to close the respective PR, because it's a breaking change which we don't want to introduce at this time. This will be fixed in the new Prisma v2 Go client (but it does not exist yet), so we're stuck with int32s for now.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

hoodsy picture hoodsy  ·  3Comments

MitkoTschimev picture MitkoTschimev  ·  3Comments

akoenig picture akoenig  ·  3Comments

tbrannam picture tbrannam  ·  3Comments

sedubois picture sedubois  ·  3Comments