-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Add support for BigInteger, Int128, and UInt128 in SqliteValueBinder #37492
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- 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.
Add support for BigInteger, Int128, and UInt128 in SqliteValueBinder
There was a problem hiding this 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.
249ae47 to
6b86657
Compare
There was a problem hiding this 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 },
};
| var value1 = (Int128)value; | ||
| BindText((value1).ToString(CultureInfo.InvariantCulture)); | ||
| } | ||
| else if (type == typeof(UInt128)) | ||
| { | ||
| var value1 = (UInt128)value; | ||
| BindText((value1).ToString(CultureInfo.InvariantCulture)); |
Copilot
AI
Feb 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable value1 is unnecessary here, and there are redundant parentheses around (value1) in the ToString call. Inline the cast directly in the ToString call as done for BigInteger on line 244, to maintain consistency.
| var value1 = (Int128)value; | |
| BindText((value1).ToString(CultureInfo.InvariantCulture)); | |
| } | |
| else if (type == typeof(UInt128)) | |
| { | |
| var value1 = (UInt128)value; | |
| BindText((value1).ToString(CultureInfo.InvariantCulture)); | |
| BindText(((Int128)value).ToString(CultureInfo.InvariantCulture)); | |
| } | |
| else if (type == typeof(UInt128)) | |
| { | |
| BindText(((UInt128)value).ToString(CultureInfo.InvariantCulture)); |
| { typeof(DateOnly), SqliteType.Text }, | ||
| { typeof(TimeOnly), SqliteType.Text }, | ||
| #endif | ||
|
|
Copilot
AI
Feb 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra blank line detected. This is inconsistent with the rest of the dictionary initialization. Remove the blank line at line 279 to maintain consistent formatting with the other entries.
| } | ||
| } | ||
|
|
||
|
|
Copilot
AI
Feb 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra blank line. Remove one of the blank lines to maintain consistent spacing between test methods throughout the file.
| #if NET7_0_OR_GREATER | ||
| else if (type == typeof(Int128)) | ||
| { | ||
| var value1 = (Int128)value; |
Copilot
AI
Feb 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing whitespace detected after the cast. The space after (Int128)value; should be removed to maintain consistent code style.
| var value1 = (Int128)value; | |
| var value1 = (Int128)value; |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
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. |
Fixes #30930