gnorm icon indicating copy to clipboard operation
gnorm copied to clipboard

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

Open freb opened this issue 6 years ago • 9 comments

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 avatar Feb 01 '19 22:02 freb

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 avatar Feb 01 '19 22:02 freb

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

frankdugan3 avatar Mar 10 '19 03:03 frankdugan3

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

natefinch avatar Mar 11 '19 18:03 natefinch

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

freb avatar Apr 10 '19 22:04 freb

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

frankdugan3 avatar Jul 03 '19 15:07 frankdugan3

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

natefinch avatar Jul 03 '19 15:07 natefinch

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.

freb avatar Feb 06 '20 19:02 freb

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

natefinch avatar Feb 09 '20 14:02 natefinch

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.

freb avatar Mar 06 '20 18:03 freb