jschaf / pggen

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

Embedded composite types using a pointer #92

Open leg100 opened 10 months ago

leg100 commented 10 months ago

Hello,

I've been using pggen for a long time in my project, OTF and it's worked supremely well. Thank you! I particularly like how you can embed composite types in results:

CREATE TABLE IF NOT EXISTS author (
  author_id  serial PRIMARY KEY,
  first_name text NOT NULL,
  last_name  text NOT NULL,
  suffix text NULL
);

CREATE TABLE IF NOT EXISTS nobel_prize_for_literature (
  year int PRIMARY KEY,
  author_id int REFERENCES author
);
-- name: FindAuthors :many
SELECT a.*, (n.*)::"nobel_prize_for_literature" AS nobel_prize_for_literature
FROM author a
LEFT JOIN nobel_prize_for_literature n USING (author_id);

from which pggen generates:

type FindAuthorsRow struct {
    AuthorID                *int32                  `json:"author_id"`
    FirstName               *string                 `json:"first_name"`
    LastName                *string                 `json:"last_name"`
    Suffix                  *string                 `json:"suffix"`
    NobelPrizeForLiterature NobelPrizeForLiterature `json:"nobel_prize_for_literature"`
}
...
// NobelPrizeForLiterature represents the Postgres composite type "nobel_prize_for_literature".
type NobelPrizeForLiterature struct {
    Year     *int32 `json:"year"`
    AuthorID *int32 `json:"author_id"`
}

Which is great. However, as you can see it's using a LEFT JOIN, because an author may not be a nobel laureate. In that case I want the embedded NobelPrizeForLiterature to instead be a pointer, *NobelPrizeForLiterature:

type FindAuthorsRow struct {
    AuthorID                *int32                  `json:"author_id"`
    FirstName               *string                 `json:"first_name"`
    LastName                *string                 `json:"last_name"`
    Suffix                  *string                 `json:"suffix"`
    NobelPrizeForLiterature *NobelPrizeForLiterature `json:"nobel_prize_for_literature"`
}

That would allow me to differentiate whether an author is a nobel laureate or not.

I therefore created a fork a long while back that uses pointers instead: https://github.com/leg100/pggen. I made no attempt to raise a PR with your project at the time (sorry). The fork has remained stale and it looks like the code in question has totally changed upstream, so much so that any attempt to rebase now results in dozens of conflicts. Which is a shame because I would like to get all the updates that have been made since.

Nonetheless, I'm in two minds as to whether this struct pointer approach is worthwhile. I can see how with a struct value, one could check the fields, e.g. Year and AuthorID, and see if they are nil or zero. Which isn't as nice I think.

What do you think? Is there a better approach?

jschaf commented 9 months ago

Nonetheless, I'm in two minds as to whether this struct pointer approach is worthwhile. I can see how with a struct value, one could check the fields, e.g. Year and AuthorID, and see if they are nil or zero. Which isn't as nice I think.

Yea, it's a tough problem to design for. The options I see:

One approach might be to extend nullability analysis to composite types. That approach is conservative and will likely have the effect of marking most composite types as nullable and using a pointer.

https://github.com/jschaf/pggen/blob/7eb2a5bb4513e1281d52e76d7f723729ff28140e/internal/pginfer/nullability.go#L13

leg100 commented 9 months ago

My preference is for the first one (Go protobuf style) because that's what has suited me just fine thus far. Of course, that would force changes on pggen users.

My concern with the nullability analysis approach is that that might make some queries return a composite type as a pointer, and as a value for other queries. And with the way I use pggen, I often have multiple queries return the exact same generated struct fields and then cast them all to my own struct. If there is a slight variation then I cannot using struct casting.

But as you say it'll mark most composite types as nullable. And if it doesn't, I could also use the --go-type option to override the nullability analysis for a particular composite type.

And I agree with your desire to avoid introducing a config file. The less configuration the better.