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

pgQuery for field returning enum fails #1601

Closed
shellscape opened this issue Mar 30, 2022 · 7 comments
Closed

pgQuery for field returning enum fails #1601

shellscape opened this issue Mar 30, 2022 · 7 comments
Labels

Comments

@shellscape
Copy link

shellscape commented Mar 30, 2022

Summary

I think this is a bug. When we use makeExtendSchema to extend a type, and attempt to return an enum, we get the error: Error: @pgQuery(...) directive called with invalid arguments - for a table value, call it with 'source' for a scalar with 'fragment'!

I'm having a hard time parsing this message, it's not very clear and could use some improvement. I also can't tell how it relates to my declaration below, or what the resolution is. Have we discovered a new bug?

Steps to reproduce

We've got a TS enum defined as:

enum Severity {
  HIGH = 'HIGH',
  LOW = 'LOW',
  MEDIUM = 'MEDIUM',
  NEUTRAL = 'NEUTRAL',
  OK = 'OK'
}

And the same declared in GraphQL:

    enum Severity {
      OK
      NEUTRAL
      LOW
      MEDIUM
      HIGH
    }

With a field declared as:

      treeShakeableLevel: Severity! @pgQuery(
        fragment: ${embed(
          (queryBuilder: any) =>
            pgSql.fragment`(
              select
              (case when (select
                (
                  coalesce(_extra->'module', _extra->'jsnext:main') is not null
                    or
                  (_extra->'type' is not null and (_extra->'type')::text = 'module')
                )
                from
                  collectors.npm_versions v
                where
                  v.npm_package_id = ${queryBuilder.getTableAlias()}.id
                and
                  v.version = ${queryBuilder.getTableAlias()}.latest_version
                and
                  v.is_deleted = false
              ) = true then ${pgSql.literal(Severity.OK)} else ${pgSql.literal(Severity.NEUTRAL )} end)
            )`
        )}
      )

Expected results

Server starts and creates schema successfully

Actual results

Error: @pgQuery(...) directive called with invalid arguments - for a table value, call it with 'source' for a scalar with 'fragment'!

Additional context

Of note, if we change Severity! to String! the server starts without issue.
Please let me know if you need any additional information or schema, resolver code, etc.

Possible Solution

n/a

@benjie
Copy link
Member

benjie commented Mar 31, 2022

You’re probably right; we probably did an isScalarType check rather than isLeafType. Fancy hunting it down?

@shellscape
Copy link
Author

shellscape commented Mar 31, 2022

Point me in the right direction and I'll give it a go. I've not worked with the repo source before so that would be helpful. (I'm unable to find that error message in source in this repo)

@shellscape
Copy link
Author

shellscape commented Mar 31, 2022

Looks like the error message exists in the graphile-engine repo. I was able to get it to compile and run by adding:

isLeafType to https://github.com/graphile/graphile-engine/blob/fb4b7f6c1680aa52105013b45b5f4a4639feb0ba/packages/graphile-utils/src/makeExtendSchemaPlugin.ts#L787

const isLeaf = isLeafType(getNamedType(type)); to https://github.com/graphile/graphile-engine/blob/fb4b7f6c1680aa52105013b45b5f4a4639feb0ba/packages/graphile-utils/src/makeExtendSchemaPlugin.ts#L888

(isScalar || isLeaf) to https://github.com/graphile/graphile-engine/blob/fb4b7f6c1680aa52105013b45b5f4a4639feb0ba/packages/graphile-utils/src/makeExtendSchemaPlugin.ts#L1065

Not 100% sure that's working correctly since I'm getting a "message": "Cannot return null for non-nullable field Package.treeShakeableLevel.", when returning that field. But it does build and run without an error.

I leave the actual fix and test(s) in your wheelhouse. This is a pretty considerable bug given that it effects enums with core functionality.

@benjie
Copy link
Member

benjie commented Apr 1, 2022

Have added this caveat to the docs: graphile/graphile.github.io@ea18be8

@shellscape
Copy link
Author

@benjie has this issue and associated fix been considered for v5?

@benjie
Copy link
Member

benjie commented Aug 5, 2023

I’ve not checked if v5 has this issue, but I doubt it does because it’s built in a completely different way.

@benjie
Copy link
Member

benjie commented Oct 4, 2023

No longer relevant because V5 works in a different way (and thus this issue almost certainly doesn't exist in V5).

@benjie benjie closed this as completed Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants