Backport 6.1 | API enhancements for vector datatype#3472
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR ports issue #3468 from the main branch to the 6.1 release, introducing a CreateNull factory for SqlVector<T>, changing vector handling to a readonly struct, and updating related components and tests.
- Added
SqlVector<T>.CreateNull(int length)and replaced constructor calls for null vectors. - Refactored
SqlVector<T>from a sealed class to a readonly struct, movedSizeto a private field, and updated interface implementations. - Updated unit and manual tests,
SqlParameter/SqlBufferhandling,SqlDataReaderlogic, reference assemblies, and documentation to support the new vector API.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/tests/UnitTests/SqlVectorTest.cs | Switched from constructor to CreateNull in null-vector tests and realigned Size assertions. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/.../NativeVectorFloat32Tests.cs | Updated test data to use vector length only, removed old size parameters, and adjusted parameter size logic. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlTypes/SqlVector.cs | Converted SqlVector<T> to a readonly struct, added CreateNull, and replaced public Size property with an internal field. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs | Ignored Size for vector parameters in setter and Prepare, and updated vector return creation to use CreateNull. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDataReader.cs | Simplified vector-to-string logic in GetFieldValueFromSqlBufferInternal. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBuffer.cs | Adjusted GetSqlVector<T> to use CreateNull for null vectors. |
| src/Microsoft.Data.SqlClient/netfx/ref/Microsoft.Data.SqlClient.cs | Updated reference API for SqlVector<T> to readonly struct and added CreateNull. |
| src/Microsoft.Data.SqlClient/netcore/ref/Microsoft.Data.SqlClient.cs | Updated reference API for SqlVector<T> to readonly struct and added CreateNull. |
| doc/snippets/Microsoft.Data.SqlTypes/SqlVector.xml | Added documentation for CreateNull and removed old constructor docs. |
| doc/snippets/Microsoft.Data.SqlClient/SqlCommand.xml | Clarified that SqlParameter.Size is ignored for vector types. |
Comments suppressed due to low confidence (3)
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/VectorTest/NativeVectorFloat32Tests.cs:20
- [nitpick] The field name
sizeInbytesdoes not follow camelCase conventions (sizeInBytes) and is now unused in tests. Consider renaming tosizeInBytesor removing this dead field to improve readability.
public static float[] testData = new float[] { 1.1f, 2.2f, 3.3f };
doc/snippets/Microsoft.Data.SqlTypes/SqlVector.xml:12
- [nitpick] The XML element tags for the constructors appear misaligned—there is a closing
</ctor1>without a matching<ctor1>and missing<ctor2>tag. Please verify that constructor documentation tags are properly opened and closed.
</ctor1>
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDataReader.cs:3366
- The special handling for non-null vectors was removed, so all vector string conversions now return the raw SqlString. This breaks the expected JSON-style output for non-null vectors. Reintroduce the null check and call
data.GetSqlVector<float>().GetString()when!data.IsNull.
if (typeof(T) == typeof(string) && metaData.metaType.SqlDbType == SqlDbTypeExtensions.Vector)
paulmedynski
left a comment
There was a problem hiding this comment.
One typo in a doc comment.
9bb1a9a to
847c4e2
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/6.1 #3472 +/- ##
==============================================
Coverage ? 68.91%
==============================================
Files ? 280
Lines ? 62322
Branches ? 0
==============================================
Hits ? 42947
Misses ? 19375
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This is a direct port of #3468 from main to release/6.1