Mark required stored-procedure parameters as NON_NULL in GraphQL#3646
Open
anushakolan wants to merge 2 commits into
Open
Mark required stored-procedure parameters as NON_NULL in GraphQL#3646anushakolan wants to merge 2 commits into
anushakolan wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Updates stored-procedure GraphQL schema generation so input argument nullability reflects whether each parameter is required, enabling clients to identify mandatory vs optional inputs via introspection (fixing #3501).
Changes:
- Wrap required stored-procedure parameter argument types in
NonNullTypeNode, defaulting to required unless config explicitly setsrequired: false. - Preserve existing default-value behavior, including allowing defaults on
NON_NULLarguments. - Add unit tests validating nullability behavior and default-value preservation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Service.GraphQLBuilder/GraphQLStoredProcedureBuilder.cs | Computes isRequired per parameter (defaulting to required) and conditionally wraps argument types in NonNullTypeNode. |
| src/Service.Tests/GraphQLBuilder/Sql/StoredProcedureBuilderTests.cs | Adds unit tests asserting correct argument nullability and that defaults are preserved for required parameters. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why make this change?
Closes #3501.
The GraphQL schema generated for stored-procedure entities emitted every input argument as nullable (
String), so introspection couldn't distinguish required parameters from optional ones. Clients calling those mutations had no way to know which arguments were mandatory until SQL Server failed the call at execution time.What is this change?
In
GenerateStoredProcedureSchema, each parameter's GraphQL type is now wrapped inNonNullTypeNodewhen the parameter is required, otherwise it stays a nullableNamedTypeNode. A parameter is treated as required unless the runtime config explicitly setsrequired: falsefor it. Names, value types, and the existingdefaultValuehandling are unchanged.A required parameter with a config-supplied default still emits
NON_NULLand keeps the default — that's valid GraphQL and matches client expectations.The default-to-required behavior is intentional: T-SQL nullability (
is_nullable) describes whether the parameter's type accepts NULL, not whether the caller must supply it. Defaulting to required produces a schema that's correct at execute time; users can opt out per-parameter via config when a procedure declares true defaults.How was this tested?
Unit tests
Added to
StoredProcedureBuilderTests:StoredProcedure_RequiredFlag_ProducesNonNullType(3 data rows):required: true→NonNullTypeNode,required: false→ nullableNamedTypeNode, parameter omitted from config → defaults toNonNullTypeNode.StoredProcedure_RequiredWithDefault_KeepsDefaultValue: required parameter with a config default emitsNonNullTypeNodeand preserves the default value.Manual end-to-end test
Three GraphQL mutations were configured against the existing
dbo.insert_book(@title varchar(max), @publisher_id int)stored procedure. Each entity exercised a different config shape, and__schemaintrospection confirmed the input argument kinds:titlepublisher_idexecuteInsertBookPartialConfigtitlelisted (required: true)executeInsertBookExplicitNotRequiredtitlerequired: false,publisher_idrequired: trueexecuteInsertBookNoConfigParamsparametersblockAll scenarios produced the expected GraphQL types.
Sample request
GraphQL introspection used to validate the schema:
{ __schema { mutationType { fields { name args { name type { kind name ofType { kind name } } } } } } }