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

Embedded composite types using a pointer #92

Open
leg100 opened this issue Oct 29, 2023 · 2 comments
Open

Embedded composite types using a pointer #92

leg100 opened this issue Oct 29, 2023 · 2 comments

Comments

@leg100
Copy link

leg100 commented Oct 29, 2023

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
Copy link
Owner

jschaf commented Nov 1, 2023

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:

  • Go protobuf style: every composite type is a pointer.

  • User configurable. The main problem here is figuring out how to specify the options. We'll want to support configuration for at least two levels:

    1. Per pggen invocation, to set default types similar to how the --go-type flag works.
    2. Per column to override. Helpful for cases like multiple JSONB columns.
    3. (Maybe) per query. This can be done as per columns, so I'd lean towards skipping it.

    With configuration, another thorny problem is figuring out how to specify the configuration. I'd really like to keep it in the SQL file instead of using a JSON or TOML file.

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.

func isColNullable(query *ast.SourceQuery, plan Plan, out string, column pg.Column) bool {

@leg100
Copy link
Author

leg100 commented Nov 2, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants