Skip to content

No longer fall back to trying all union members when the variant selected by discriminator fails to serialize#12825

Merged
Viicos merged 4 commits intopydantic:mainfrom
navalprakhar:fix/12800-tagged-union-serialization-fallback
Mar 27, 2026
Merged

No longer fall back to trying all union members when the variant selected by discriminator fails to serialize#12825
Viicos merged 4 commits intopydantic:mainfrom
navalprakhar:fix/12800-tagged-union-serialization-fallback

Conversation

@navalprakhar
Copy link
Copy Markdown
Contributor

@navalprakhar navalprakhar commented Feb 20, 2026 •
edited by davidhewitt
Loading

Change Summary

Don't try every union member when the discriminator already found the right one but serialization failed.

Related issue number

Closes pydantic/pydantic-core#1455
fix #12800
Ref #12822 (reduces spurious warnings from N to 1 by not falling back to all union members)

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Selected Reviewer: @Viicos

@github-actions github-actions Bot added the relnotes-fix Used for bugfixes. label Feb 20, 2026
@navalprakhar
Copy link
Copy Markdown
Contributor Author

navalprakhar commented Feb 20, 2026 •
edited
Loading

Note: the Build WASM Emscripten failure appears to be unrelated. The same snapshot test (test_set_frozenset[frozenset_schema]) is also failing on main.

Fixed by #12837 on main; rebased.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Feb 20, 2026 •
edited
Loading

Merging this PR will not alter performance

✅ 212 untouched benchmarks

Comparing navalprakhar:fix/12800-tagged-union-serialization-fallback (ad3f375) with main (1a37a3d)

Open in CodSpeed

@navalprakhar
Copy link
Copy Markdown
Contributor Author

navalprakhar commented Feb 20, 2026

please review

This was referenced Feb 20, 2026
Closed
Closed
Closed
@navalprakhar navalprakhar force-pushed the fix/12800-tagged-union-serialization-fallback branch from 49f539e to 9c46ac4 Compare February 20, 2026 12:08
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 2, 2026 •
edited
Loading

Coverage report

This PR does not seem to contain any modification to coverable code.

@navalprakhar navalprakhar changed the title Don't try all union members when tagged discriminator match fails Skip trying all union members when the matched variant fails to serialize Mar 2, 2026
davidhewitt
davidhewitt reviewed Mar 9, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, and sorry for the slow reply.

I have been thinking about this PR and discussing with @Viicos. I believe (@Viicos please correct me if I am wrong) that while this is slightly breaking because it removes the left-to-right path from this serialization, it feels almost certain that if the tag selected a variant which then fails to serialize, the other union members will fail to serialize too.

There might be some edge case out there where the left-to-right serialization was succeeding, but falling back to type inference is probably good enough for most of those, and if the left-to-right serialization was really necessary we can encourage users to use a TypeAdapter in left-to-right serialization mode to define their own PlainSerializer.

We should make sure that this is mentioned in the release notes for 2.13.

Comment thread pydantic-core/src/serializers/type_serializers/union.rs Outdated
Comment thread pydantic-core/src/serializers/type_serializers/union.rs Outdated
@navalprakhar navalprakhar force-pushed the fix/12800-tagged-union-serialization-fallback branch 4 times, most recently from 2866635 to 219b6c9 Compare March 16, 2026 17:18
davidhewitt
davidhewitt reviewed Mar 18, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Thanks, I think we can simplify the test and then merge.

Comment thread pydantic-core/tests/serializers/test_union.py Outdated
@davidhewitt davidhewitt added needs-blogpost-entry This PR needs to be documented in the release notes blog post relnotes-change Used for changes to existing functionality which don't have a better categorization. and removed relnotes-fix Used for bugfixes. labels Mar 18, 2026
@davidhewitt davidhewitt changed the title Skip trying all union members when the matched variant fails to serialize No longer fall back to trying all union members when the variant selected by discriminator fails to serialize Mar 18, 2026
@navalprakhar navalprakhar force-pushed the fix/12800-tagged-union-serialization-fallback branch from 219b6c9 to 0384c08 Compare March 18, 2026 12:54
Viicos
Viicos reviewed Mar 25, 2026
View reviewed changes
Comment thread pydantic-core/src/serializers/type_serializers/union.rs
Comment thread pydantic-core/tests/serializers/test_union.py Outdated
@navalprakhar navalprakhar force-pushed the fix/12800-tagged-union-serialization-fallback branch from 0384c08 to 9780fc4 Compare March 25, 2026 22:37
@navalprakhar navalprakhar force-pushed the fix/12800-tagged-union-serialization-fallback branch from 9780fc4 to ad3f375 Compare March 25, 2026 22:42
@navalprakhar navalprakhar requested a review from Viicos March 25, 2026 22:57
@Viicos Viicos merged commit 870ebe8 into pydantic:main Mar 27, 2026
81 checks passed
@navalprakhar navalprakhar deleted the fix/12800-tagged-union-serialization-fallback branch March 27, 2026 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Viicos Viicos Viicos approved these changes

@davidhewitt davidhewitt Awaiting requested review from davidhewitt

Assignees

@Viicos Viicos

Labels

needs-blogpost-entry This PR needs to be documented in the release notes blog post ready for review relnotes-change Used for changes to existing functionality which don't have a better categorization.

Projects

None yet

Milestone

No milestone

3 participants

@navalprakhar @davidhewitt @Viicos