jschaf / pggen

Generate type-safe Go for any Postgres query. If Postgres can run the query, pggen can generate code for it.
MIT License
282 stars 26 forks source link

Would it be possible to make Postgres `bigint` map to Go `int64` by default? #36

Closed djsavvy closed 2 years ago

djsavvy commented 3 years ago

Currently, it maps to int, which may not actually be 64 bits.

A workaround is for me to pass in --go-type bigint=*int64, but this converts all the non-null bigint columns into *int64s as well instead of int64. If I pass in --go-type bigint=int64, then I lose nullability altogether.

EDIT: Is it intended behavior that trying to map a bigint to an int64 means that all bigint columns (nullable and not-null) become int64 instead of nullable ones becoming *int64 and non-null ones becoming int64?

jschaf commented 3 years ago

Would it be possible to make Postgres bigint map to Go int64 by default?

That's admittedly more correct, but less ergonomic (assuming the majority of platforms are 64 bit platforms). I happen to like int so I'm loathe to change it now.

Currently, it maps to int, which may not actually be 64 bits.

Gotcha, I'm used to beefy servers. What platforms does int map to something other than 64 bits?

Is it intended behavior that trying to map a bigint to an int64 means that all bigint columns (nullable and not-null) become int64 instead of nullable ones becoming *int64 and non-null ones becoming int64?

Yes, there's a couple problems with specifying nullability:

  1. A minor concern is there's not a great way to support specifying a null and non-null type with a flag. I could use a comma delimited flag, e.g. --go-type bigint=*int64,int64

  2. It's not possible to infer whether an output column is nullable in anything other than simple cases. Consider SELECT foo FROM some_black_box_function()? pggen can get the type of foo but Postgres doesn't report nullability, so pggen can't know whether foo is nullable or not. I did some incredibly basic null checking in pginfer/nullability but it's hard to cover any advanced usecases. I think I'd need a real SQL parser and control-flow graph which sounds hard.

djsavvy commented 3 years ago

The nullability concerns make sense to me --- thanks for the explanation.

What platforms does int map to something other than 64 bits?

This would occur on a 32-bit system, for instance.

jschaf commented 2 years ago

This ship has sailed; it's too late to switch from int to int64. A workaround is --go-type bigint=*int64. This would be improved by allowing the user to specify the nullable and non-nullable types.