search

gnormal / gnorm

A database-first code generator for any language
https://gnorm.org
Other
487 stars 40 forks source link

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

Open freb opened 5 years ago

freb commented 5 years ago

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: https://github.com/gnormal/gnorm/blob/d6968144f5a051edae8507f1bd46f87fb53d7a6c/database/drivers/postgres/parse.go#L599-L605

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 commented 5 years ago

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)
frankdugan3 commented 5 years ago

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

natefinch commented 5 years ago

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

freb commented 5 years ago

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

frankdugan3 commented 5 years ago

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

natefinch commented 5 years ago

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

freb commented 4 years ago

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 commented 4 years ago

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

freb commented 4 years ago

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.