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

Schema.Enums contains one value for each value instead of each type #115

Open
freb opened this issue Feb 1, 2019 · 9 comments
Open

Schema.Enums contains one value for each value instead of each type #115

freb opened this issue Feb 1, 2019 · 9 comments

Comments

@freb
Copy link
Contributor

freb commented Feb 1, 2019

As the title says, when ranging .Schema.Enums in a [SchemaPaths] template, each enum is repeated once for each associated value, when it should just range once for each enum type. You can see this also in the verbose output when running gnorm:

found 3 values for enum public.project_type
found 3 values for enum public.project_type
found 3 values for enum public.project_type
found 2 values for enum public.stage_type
found 2 values for enum public.stage_type

This may be PostgreSQL only, as I think the culprit is the following query:

SELECT n.nspname, t.typname as type
FROM pg_type t
LEFT JOIN pg_catalog.pg_namespace n ON n.oid = t.typnamespace
JOIN pg_enum e ON t.oid = e.enumtypid
WHERE (t.typrelid = 0 OR (SELECT c.relkind = 'c' FROM pg_catalog.pg_class c WHERE c.oid = t.typrelid))
AND NOT EXISTS(SELECT 1 FROM pg_catalog.pg_type el WHERE el.oid = t.typelem AND el.typarray = t.oid)
AND n.nspname IN (%s)`

I would have submitted a PR, but I'm not great at SQL and can't figure out how to adjust the query. The results could obviously be de-duplicated in code, but that seems like a second choice solution.

My hunch as to why this went unnoticed is that when the values generated are passed to [EnumPaths] templates, generally one file is created per enum and you wouldn't know if they were written multiple times.

@freb
Copy link
Contributor Author

freb commented Feb 1, 2019

It actually seems to work fine if you just omit the join on pg_enum so that the query looks like:

SELECT      n.nspname, t.typname as type
FROM        pg_type t
LEFT JOIN   pg_catalog.pg_namespace n ON n.oid = t.typnamespace
WHERE       (t.typrelid = 0 OR (SELECT c.relkind = 'c' FROM pg_catalog.pg_class c WHERE c.oid = t.typrelid))
AND     NOT EXISTS(SELECT 1 FROM pg_catalog.pg_type el WHERE el.oid = t.typelem AND el.typarray = t.oid)
AND         n.nspname = IN (%s)

freb added a commit to freb/gnorm that referenced this issue Feb 27, 2019
@frankdugan3
Copy link

Running into this issue right now, would love to see the fix merged in!

@natefinch
Copy link
Member

Sorry for the delay, I'll take a look tonight and try to get that PR merged in.

@freb
Copy link
Contributor Author

freb commented Apr 10, 2019

Sorry for the PR noise. I had to do some branch cleanups to use this locally.

@frankdugan3
Copy link

Hate to be that guy, but... bump?

@natefinch
Copy link
Member

Gah, sorry. I appreciate the bump. I'll try to look at this tonight.

@freb
Copy link
Contributor Author

freb commented Feb 6, 2020

Hi @natefinch, is there something else you'd like to see for the PR? I can add a test case to preview_test.go if that's the hold up. Just let me know what you need.

@natefinch
Copy link
Member

A test would be great, thank you! Sorry for the delay in responding!

@freb
Copy link
Contributor Author

freb commented Mar 6, 2020

After looking at this again, I realize a test case in preview_test.go won't help here. The issue is in the query from the database that results in duplicate enums. The solution is to fix the query, not to check for duplicates in code. So we would expect adding duplicate enums to Parse in preview_test.go would also be reflected in the JSON used for comparison.

I've been using this in my fork for a long time now without issue. If you're using enums for anything other than producing one file per, such as with [EnumPaths], then gnorm will currently produce wrong code.

This issue will manifest in the grpc template I'm about to push. The example SQL (which I copied from postgres-go) only has a single enum, so won't be affected. But anyone using the template that has more than one enum in the schema will have invalid code produced.

Let me know what else you need to get this merged.

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

3 participants