Skip to content

Conversation

@claythearc
Copy link

@claythearc claythearc commented Oct 29, 2025

Relates to: #835

Quick little fix for a recursion bug when annotating a member as a Dictionary or Mapping.

claude and others added 3 commits October 29, 2025 15:32
Fixed a bug where using Dict or Mapping type hints with Relationship()
would cause infinite recursion. The get_relationship_to() function now
properly handles dict/Mapping types by extracting the value type (second
type argument), similar to how it handles List types.

Changes:
- Added handling for dict and Mapping origins in get_relationship_to()
- Extracts the value type from Dict[K, V] or Mapping[K, V] annotations
- Added comprehensive tests for Dict relationships with attribute_mapped_collection

This resolves the RecursionError that occurred when defining relationships
like: children: Dict[str, Child] = Relationship(...)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
…cursion-011CUbiwV1LogX5vM5VcjpbW

Fix RecursionError with Dict relationships in get_relationship_to()
@YuriiMotov
Copy link
Member

We already have #1287 that fixes this issue.
But, it's good idea to check that key and value type are specified

class Parent(SQLModel, table=True):
__tablename__ = "parent"
id: int = Field(primary_key=True)
children: Optional[Dict[str, "Child"]] = Relationship(
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to use Optional here?

Copy link
Author

Choose a reason for hiding this comment

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

I think you can make a reasonable argument either way. After going back and forth with Claude some, I decided that it made sense because it lets you distinguish between not loaded and empty, but I dont have exceptionally strong feelings and can change it.

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.

3 participants