-
Notifications
You must be signed in to change notification settings - Fork 311
[6.1] Vector ref assembly fixes and enable vector and json tests #3562
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
[6.1] Vector ref assembly fixes and enable vector and json tests #3562
Conversation
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
This PR ports vector-related API fixes and enables test coverage for vector and JSON functionality by making them available on Azure SQL. The changes update reference assemblies to fix the nullable return type for SqlVector<T>.Null
and enable previously disabled vector and JSON tests.
- Fixes the return type of
SqlVector<T>.Null
property from non-nullable to nullable in reference assemblies - Enables vector and JSON test support by updating feature flags to be based on Azure SQL detection
- Adds a new validation test for SqlVector API functionality in manual tests
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/VectorTest/VectorAPIValidationTest.cs |
New test file for validating SqlVector reference assembly APIs |
src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj |
Reorganizes test file inclusions, moving vector and JSON tests from excluded to included |
src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs |
Updates feature flags to enable JSON and Vector support for Azure SQL |
src/Microsoft.Data.SqlClient/netfx/ref/Microsoft.Data.SqlClient.cs |
Fixes SqlVector Null property return type to be nullable |
src/Microsoft.Data.SqlClient/netcore/ref/Microsoft.Data.SqlClient.cs |
Fixes SqlVector Null property return type to be nullable |
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/VectorTest/VectorAPIValidationTest.cs
Show resolved
Hide resolved
Assert.Equal(36, (int)SqlDbTypeExtensions.Vector); | ||
|
||
// Validate ctor1 with float[] : public SqlVector(System.ReadOnlyMemory<T> memory) { } | ||
var vector = new SqlVector<float>(VectorFloat32TestData.testData); |
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.
Nit: Sharing test data across classes like this can make it hard to update tests in the future. It's fine to replicate simple test data. This method could also be broken out into individual tests for the float[] constructor, ROM constructor, null vector, and create null method.
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.
@apoorvdeshmukh please address this in 'main'. I will proceed with this PR in release/6.1 branch.
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## release/6.1 #3562 +/- ##
===============================================
- Coverage 69.69% 61.04% -8.66%
===============================================
Files 281 275 -6
Lines 62413 62099 -314
===============================================
- Hits 43500 37907 -5593
- Misses 18913 24192 +5279
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:
|
Description
Ports #3521 and enable vector and json test coverage through port of #3559.
Testing
Adds a testcase for validation of ref assembly APIs for SqlVector.
Guidelines
Please review the contribution guidelines before submitting a pull request: