Skip to content

fix(adonet): normalize integer reads - #11283

Merged
ReubenBond merged 1 commit into
dotnet:mainfrom
ReubenBond:rb-fix-adonet-normalize-integer-reads
Sep 16, 2026
Merged

ReubenBond merged 1 commit into
dotnet:mainfrom
ReubenBond:rb-fix-adonet-normalize-integer-reads

Conversation

@ReubenBond

@ReubenBond ReubenBond commented Sep 16, 2026 •

Copy link
Copy Markdown
Member

Problem

ADO.NET providers can return integer-valued columns as byte, short, long, or decimal. The shared named-column GetInt32 helper delegates to the provider's typed getter, which can reject these representations. Nullable integer conversion also depends on the ambient culture.

Solution

Convert the provider-returned value with Convert.ToInt32(value, CultureInfo.InvariantCulture) in both integer helpers. This gives required and nullable reads consistent numeric conversion and overflow behavior while preserving nullable DBNull handling and missing-field exception context.

The change is scoped to the existing internal DbExtensions implementation, its XML summaries, and one provider-independent regression test class. The tests cover exact numeric values, range boundaries, invariant culture, database nulls, invalid conversions, and missing-field context. The public API remains unchanged.

Attribution

Extracted from @KSemenenko's #9903, specifically commit 0c579afaa1a6ff8aefecf7b8be06cb5ef790f7ff, with additional focused regression coverage. Preserved source head: ef0f35790de3f6730215319964ad2abdb6605e50; source integration base: 558213beed6bf2aecbec59d2d3458810e689912e.

This independent fix is based directly on main at 11333532a501f73c7bc824e07dc3693aa93c3b0a. #9903 remains the preserved source and attribution reference.

Microsoft Reviewers: Open in CodeFlow

Convert provider-returned values to Int32 with invariant culture for required and nullable reads. Preserve database-null handling and missing-field context, and cover exact values, range boundaries, culture, and conversion exceptions.

Extract the independent numeric-conversion fix and its focused test class from the preserved advanced-reminder proposal, with additional regression coverage and helper documentation.

Co-authored-by: ksemenenko <mail@ksemenenko.com>
Source-Commit: 0c579af
Source-Pull-Request: dotnet#9903
Source-Head: ef0f357
Source-Base: 558213b
Copilot AI lite review requested due to automatic review settings September 16, 2026 21:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The focused implementation preserves existing exception and null semantics and is comprehensively regression-tested.

Review effort: Lite
Findings: None

What changed in this PR

Normalizes ADO.NET integer reads across provider-specific numeric types using invariant conversion while preserving null and missing-field behavior.

Changes:

  • Updated required and nullable GetInt32 helpers.
  • Added comprehensive provider-independent regression tests.
File Description
src/​AdoNet/​Shared/​Storage/​DbExtensions.cs Applies invariant numeric conversion.
test/​Extensions/​Orleans.AdoNet.Tests/​StorageTests/​DbExtensionsInt32ConversionTests.cs Covers conversions, boundaries, errors, culture, and nulls.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

Copy link
Copy Markdown
Contributor

Code coverage

Metric Pull request Current main Variance
Lines 82.08% (112,073 / 136,545) 82.07% (112,069 / 136,545) +0.0029 pp
Branches 71.23% (32,212 / 45,220) 71.22% (32,204 / 45,220) +0.0177 pp

Report-only conclusion: improved.

The current-main baseline is commit 11333532a5 and uses the same reviewed coverage matrix.

Coverage combines every CI test matrix job, including providers, CodeGen, .NET 8/10, Linux, Windows, and macOS, using canonical physical source and branch identities.

The comparison remains report-only while normal line and branch variance is calibrated.

Coverage details

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.

2 participants