Skip to content

Only reuse memoized default values for the type they were coerced to - #279

Open
patrick91 wants to merge 1 commit into
graphql-python:mainfrom
patrick91:fix-default-memoization-per-type
Open

patrick91 wants to merge 1 commit into
graphql-python:mainfrom
patrick91:fix-default-memoization-per-type

Conversation

@patrick91

@patrick91 patrick91 commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

I found this one while doing some changes in Strawberry, it's a small bug, @Cito the fix looks ok to me, but not sure if it is the way you'd approach it, so feel free to change it :D

Basically when a GraphQLDefaultInput is used with more than one type, which happens with extend_schema, every type gets the default value coerced for whichever type was queried first.

Description from Claude:

coerce_default_value memoizes the coerced default on the GraphQLDefaultInput object, but the same object can be used with more than one type, and the memoized value was then reused for all of them: whichever type was coerced first decided the default for the others.

extend_schema runs into this, as it keeps the default of existing arguments and input fields but replaces their types, so the original and the extended schema share the memoized value:

from graphql import build_schema, extend_schema, graphql_sync, parse

schema = build_schema("""
    type Query {
      someInput(arg: SomeInput = {}): String
    }

    input SomeInput {
      oldField: String
    }
""")
extended_schema = extend_schema(
    schema, parse('extend input SomeInput { newField: String = "new" }')
)
root_value = {"someInput": lambda _info, arg: str(arg)}

graphql_sync(schema, "{ someInput }", root_value)  # {'someInput': '{}'}
graphql_sync(extended_schema, "{ someInput }", root_value)  # {'someInput': '{}'}, expected "{'newField': 'new'}"

Querying the extended schema first gives {'newField': 'new'} to the original schema instead. The same happens when one GraphQLDefaultInput is passed to arguments of different types. This gist runs that case with graphql-core 3.3.0 and graphql-js 17.0.2, which memoizes on the argument and isn't affected: https://gist.github.com/patrick91/f4c61c55dea911a9da311ad724e03f99

The fix keeps the memoization on GraphQLDefaultInput, so it still works for the variable signatures of fragment arguments, but stores the type the value was coerced to along with it, and only reuses the value for that type. Both are stored as one tuple, so concurrent coercions can't pair a type with another type's value.

Tests: memoizes_coercion_per_type in test_coerce_input_value.py and coerces_default_values_with_extended_input_types in test_extend_schema.py, which both fail without the change. The full test suite (also on Python 3.10), the doctests of both modules, ruff check src tests, ruff format --check src tests and mypy src tests pass.

I found this while looking into how defaults are shared between requests for strawberry-graphql/strawberry#4660 (Strawberry still uses default_value, so it isn't affected).

AI: I've done this with Claude.

@patrick91
patrick91 requested a review from Cito as a code owner October 6, 2026 17:43
@codspeed

codspeed Bot commented Oct 6, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 20 untouched benchmarks


Comparing patrick91:fix-default-memoization-per-type (62574cc) with main (bdab43b)

Open in CodSpeed

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.

1 participant