Fix delayed converter resolution for nullables#6453
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request fixes an issue where delayed converter resolution for nullable arrays fails when null values precede non-null values. The problem occurred because null values in nullable arrays (e.g., DateTime?[]) were locking in the default type (timestamp), preventing subsequent non-null values that require a different type (timestamptz) from being properly resolved.
Changes:
- Fixed
NullableConverterto return null instead of the default resolution for null values, allowing array converter to continue looking for non-null values - Simplified
ArrayConverterby removing special null handling (but introduces a regression) - Added test case to verify null values can precede non-null values in nullable arrays
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Npgsql/Internal/Converters/NullableConverter.cs | Changed GetEffectiveResolution to return null for null values instead of getting default resolution, enabling delayed resolution |
| src/Npgsql/Internal/Converters/ArrayConverter.cs | Removed special handling for null arrays and simplified switch statement structure (introduces bug for null arrays) |
| test/Npgsql.Tests/Types/DateTimeTests.cs | Added test case verifying that null can precede non-null DateTimeKind.Utc values in DateTime?[] arrays |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This would show up if we would ever support jagged arrays
3ff2bc9 to
d399a7c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
(cherry picked from commit 013e771)
(cherry picked from commit 013e771)
(cherry picked from commit 013e771)
Closes #6450