Skip to content

fix: remove double NON_NULL wrapping on byPk argument types#635

Merged
utkarash2991 merged 1 commit into
masterfrom
utkarashsingh/633-fix-double-non-null-bypk-args
May 7, 2026
Merged

fix: remove double NON_NULL wrapping on byPk argument types#635
utkarash2991 merged 1 commit into
masterfrom
utkarashsingh/633-fix-double-non-null-bypk-args

Conversation

@utkarash2991
Copy link
Copy Markdown
Contributor

Primary key columns are already wrapped in NonNull by sql_column_to_graphql_type for NOT NULL columns. The byPk argument construction was wrapping a second time unconditionally, producing NON_NULL(NON_NULL(Scalar)) which violates the GraphQL spec and breaks GraphiQL, Prisma, and code generators.

Fix by checking if col_type is already NonNull before wrapping. View primary key columns (always nullable in PG) still get wrapped once.

Fixes #633

What kind of change does this PR introduce?

Bug fix

Copy link
Copy Markdown
Contributor

@imor imor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving to unblock. @utkarash2991 were you able to reproduce with GraphiQL?

Primary key columns are already wrapped in NonNull by
sql_column_to_graphql_type for NOT NULL columns. The byPk argument
construction was wrapping a second time unconditionally, producing
NON_NULL(NON_NULL(Scalar)) which violates the GraphQL spec and breaks
GraphiQL, Prisma, and code generators.

Fix by checking if col_type is already NonNull before wrapping. View
primary key columns (always nullable in PG) still get wrapped once.

Fixes #633
@utkarash2991 utkarash2991 force-pushed the utkarashsingh/633-fix-double-non-null-bypk-args branch from 0622602 to 74babad Compare May 7, 2026 11:01
@utkarash2991
Copy link
Copy Markdown
Contributor Author

were you able to reproduce with GraphiQL?
@imor and I verified locally that the fix works for the GraphiQL. Proceeding ahead with merge.

Copy link
Copy Markdown
Contributor

@imor imor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to go, let's merge it.

@utkarash2991 utkarash2991 merged commit 151ee26 into master May 7, 2026
8 checks passed
@utkarash2991 utkarash2991 deleted the utkarashsingh/633-fix-double-non-null-bypk-args branch May 7, 2026 11:07
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

Successfully merging this pull request may close these issues.

Wrong introspection schema response

2 participants