Skip to content

Add support for BigInteger, Int128, and UInt128 in SqliteValueBinder#37492

Merged
AndriySvyryd merged 37 commits into
dotnet:mainfrom
MaikyOzr:main
May 27, 2026
Merged

Add support for BigInteger, Int128, and UInt128 in SqliteValueBinder#37492
AndriySvyryd merged 37 commits into
dotnet:mainfrom
MaikyOzr:main

Conversation

@MaikyOzr
Copy link
Copy Markdown
Contributor

@MaikyOzr MaikyOzr commented Jan 12, 2026 •
edited by AndriySvyryd
Loading

  • I've read the guidelines for contributing and seen the walkthrough
  • I've posted a comment on an issue with a detailed description of how I am planning to contribute and got approval from a member of the team
  • The code builds and tests pass locally (also verified by our automated build checks)
  • Commit messages follow this format:
        Summary of the changes
        - Detail 1
        - Detail 2

        Fixes #bugnumber
  • Tests for the changes have been added (for bug fixes / features)
  • Code follows the same patterns and style as existing code in this repo

@MaikyOzr
Add support for BigInteger, Int128, and UInt128 in SqliteValueBinder
6505fd5 
- Bind Int128, UInt128, and BigInteger parameters as TEXT in SQLite.
- Add provider-level tests to verify correct binding of these large numeric types.
- This change only affects parameter binding; reading values back still returns string.
@MaikyOzr
Merge pull request #1 from MaikyOzr/Support-BigInteger-MaikyOzr
468c26b 
Add support for BigInteger, Int128, and UInt128 in SqliteValueBinder
@MaikyOzr
Merge branch 'dotnet:main' into main
5695bf6
@MaikyOzr MaikyOzr requested a review from a team as a code owner January 12, 2026 22:02
@AndriySvyryd AndriySvyryd requested a review from Copilot January 12, 2026 23:19
Copilot AI reviewed Jan 12, 2026
View reviewed changes
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@roji roji force-pushed the main branch 2 times, most recently from 249ae47 to 6b86657 Compare January 13, 2026 17:46
@AndriySvyryd AndriySvyryd requested a review from Copilot February 4, 2026 23:08
Copilot AI reviewed Feb 4, 2026
View reviewed changes
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

test/Microsoft.Data.Sqlite.Tests/SqliteParameterTest.cs:670

  • The new types BigInteger, Int128, and UInt128 should be added to the TypesData collection to ensure comprehensive test coverage via the existing SqliteType_is_inferred_from_value test. This would follow the pattern established for other supported types like decimal (line 646) and provide additional test coverage beyond the specific binding tests.
    public static IEnumerable<object[]> TypesData
        => new List<object[]>
        {
            new object[] { default(DateTime), SqliteType.Text },
            new object[] { default(DateTimeOffset), SqliteType.Text },
            new object[] { DBNull.Value, SqliteType.Text },
            new object[] { 0m, SqliteType.Text },
            new object[] { default(Guid), SqliteType.Text },
            new object[] { default(TimeSpan), SqliteType.Text },
            new object[] { default(TimeSpan), SqliteType.Text },
#if NET6_0_OR_GREATER
            new object[] { default(DateOnly), SqliteType.Text },
            new object[] { default(TimeOnly), SqliteType.Text },
#endif
            new object[] { 'A', SqliteType.Text },
            new object[] { "", SqliteType.Text },
            new object[] { false, SqliteType.Integer },
            new object[] { (byte)0, SqliteType.Integer },
            new object[] { 0, SqliteType.Integer },
            new object[] { 0L, SqliteType.Integer },
            new object[] { (sbyte)0, SqliteType.Integer },
            new object[] { (short)0, SqliteType.Integer },
            new object[] { 0u, SqliteType.Integer },
            new object[] { 0ul, SqliteType.Integer },
            new object[] { (ushort)0, SqliteType.Integer },
            new object[] { 0.0, SqliteType.Real },
            new object[] { 0f, SqliteType.Real },
            new object[] { Array.Empty<byte>(), SqliteType.Blob },
            new object[] { new Memory<byte>([]), SqliteType.Blob },
            new object[] { new ReadOnlyMemory<byte>([]), SqliteType.Blob },
        };

Comment thread src/Microsoft.Data.Sqlite.Core/SqliteValueBinder.cs Outdated
Comment thread src/Microsoft.Data.Sqlite.Core/SqliteValueBinder.cs Outdated
Comment thread src/Microsoft.Data.Sqlite.Core/SqliteValueBinder.cs
Comment thread test/Microsoft.Data.Sqlite.Tests/SqliteParameterTest.cs Outdated
Comment thread src/Microsoft.Data.Sqlite.Core/SqliteValueBinder.cs Outdated
@MaikyOzr
Update src/Microsoft.Data.Sqlite.Core/SqliteValueBinder.cs
17b9daf 
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@MaikyOzr
Copy link
Copy Markdown
Contributor Author

MaikyOzr commented Feb 5, 2026

@copilot open a new pull request to apply changes based on the comments in this thread

@MaikyOzr
Copy link
Copy Markdown
Contributor Author

MaikyOzr commented Feb 5, 2026

The current .NET SDK does not support targeting .NET 11.0. Either target .NET 10.0 or lower, or use a version of the .NET SDK that supports .NET 11.0. Download the .NET SDK from https://aka.ms/dotnet/download

.NET 11 SDK is not available for download on the official Microsoft site, so I cannot install it. Please advise which supported .NET version should be used instead.

@AndriySvyryd AndriySvyryd assigned AndriySvyryd and unassigned roji May 26, 2026
AndriySvyryd
AndriySvyryd requested changes May 27, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@AndriySvyryd AndriySvyryd left a comment

Choose a reason for hiding this comment

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

Mapping these types as TEXT doesn’t preserve ordering unless the values are padded. So, remove BigInteger support and pad Int128 and UInt128 values to 39 digits when converting to a string.

AndriySvyryd
AndriySvyryd reviewed May 27, 2026
View reviewed changes
Comment thread src/Microsoft.Data.Sqlite.Core/SqliteValueBinder.cs Outdated
#if NET7_0_OR_GREATER
else if (type == typeof(Int128))
{
var shifted = (UInt128)((Int128)value - Int128.MinValue);
Copy link
Copy Markdown
Member

@AndriySvyryd AndriySvyryd May 27, 2026

Choose a reason for hiding this comment

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

You are right that the lexicographical order wouldn't work for negative numbers, but while this produces the correct order concatenating the value with a string will produce an incorrect and unexpected result.
You'll have to drop the support for Int128 as well from this PR.
Also add tests for ordering and concatenation

Copy link
Copy Markdown
Contributor Author

@MaikyOzr MaikyOzr May 27, 2026

Choose a reason for hiding this comment

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

Can you tell me why I need to do this? (You'll have to remove support for Int128 from this PR as well.)

Copy link
Copy Markdown
Member

@AndriySvyryd AndriySvyryd May 27, 2026

Choose a reason for hiding this comment

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

Because some operations (like ||) would produce incorrect results. If the user is ok with that they should opt-in explicitly by converting it to a string themselves, but we shouldn't pretend like this is a supported data type that will just work

Copy link
Copy Markdown
Contributor Author

@MaikyOzr MaikyOzr May 27, 2026

Choose a reason for hiding this comment

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

Does that mean I can just close the PR?

Copy link
Copy Markdown
Member

@AndriySvyryd AndriySvyryd May 27, 2026

Choose a reason for hiding this comment

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

You could. But I don't see any significant problems with the UInt128 support

@AndriySvyryd AndriySvyryd merged commit 2629b33 into dotnet:main May 27, 2026
13 checks passed
@AndriySvyryd
Copy link
Copy Markdown
Member

AndriySvyryd commented May 27, 2026

Thanks for your contribution!

@dotnet-maestro dotnet-maestro Bot mentioned this pull request May 28, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@AndriySvyryd AndriySvyryd AndriySvyryd approved these changes

Assignees

@AndriySvyryd AndriySvyryd

Labels

community-contribution

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@MaikyOzr @AndriySvyryd @roji