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

omit_unused_structs generates duplicate enum code #2445

Closed
josharian opened this issue Jul 12, 2023 · 6 comments · Fixed by #2447
Closed

omit_unused_structs generates duplicate enum code #2445

josharian opened this issue Jul 12, 2023 · 6 comments · Fixed by #2447

Comments

@josharian
Copy link
Contributor

josharian commented Jul 12, 2023

Version

Other

What happened?

v1.19.0 (not an option in the dropdown)

I added omit_unused_structs to our sqlc.yaml.

The resulting generated code included duplicate definitions for several enums, including type def, constants, methods, causing a compilation failure.

$ grep "^type" models.go | sort | uniq -c | grep -v 1 
   2 type CookingType string
   2 type HomeOwnershipType string
   2 type NullCookingType struct {
   2 type NullHomeOwnershipType struct {
   2 type NullSpaceHeatingType struct {
   2 type NullWaterHeatingType struct {
   2 type SpaceHeatingType string
   2 type WaterHeatingType string

Unfortunately, I can't reproduce using a trivial example in the playground, and I'm out of debugging time, so I'm filing this in the hopes that the problem is obvious from the symptoms.

Relevant log output

// none

Database schema

CREATE TYPE cooking_type AS ENUM(
	'gas',
	'electric',
	'induction',
	'other',
	'unknown'
);

SQL queries

No response

Configuration

version: 1
packages:
  - path: "."
    name: "sqlc"
    sql_package: "pgx/v5"
    engine: "postgresql"
    schema: "../migration"
    queries: // redacted
    emit_exact_table_names: true
    emit_enum_valid_method: true
    emit_all_enum_values: true
    emit_result_struct_pointers: true
    emit_params_struct_pointers: true
    omit_unused_structs: true

Playground URL

https://play.sqlc.dev/p/ec4ad0299744dc223ce2402ec06836dedc94d4cf2cf7727950c9e0f284132b44

What operating system are you using?

macOS

What database engines are you using?

PostgreSQL

What type of code are you generating?

Go

@josharian josharian added bug Something isn't working triage New issues that hasn't been reviewed labels Jul 12, 2023
@kyleconroy kyleconroy added 📚 postgresql 🔧 golang 💻 darwin pgx/v5 and removed triage New issues that hasn't been reviewed labels Jul 13, 2023
@kyleconroy kyleconroy added this to the v1.19.1 milestone Jul 13, 2023
@kyleconroy
Copy link
Collaborator

I've attempted to reproduce here, no luck yet

@josharian
Copy link
Contributor Author

drat. OK, i'll see about bisecting my way down to a reproducer. it may take me a while, there's a lot of code to chuck.

@josharian
Copy link
Contributor Author

the good news is that that pull request fixes the issue. the bad news is that it exposes a new issue. have to eat dinner now, will file a new issue tomorrow.

@andrewmbenton
Copy link
Collaborator

Thanks for taking the time to try this @josharian. If the newly-exposed issue is related to duplicate enums feel free to reopen.

@josharian
Copy link
Contributor Author

If the newly-exposed issue is related to duplicate enums feel free to reopen.

It turns out the new issue is that we were using one of the newly garbage collected enums in a jsonb schema definition Go struct type. However, we were not using it in any actual database table schemas. As a consequence the Go type for the enum got garbage collected. That seems pretty understandable. And the workaround is easy, if unfortunate: add a dummy placeholder that uses the enum type.

@josharian
Copy link
Contributor Author

Actually, the placeholder db table isn't working for me well either. Oh well, I'll just live without this flag in that one spot for now. It's already a major improvement in the other places we use it.

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

Successfully merging a pull request may close this issue.

3 participants