Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Support vector datatype with SqlVector<T>#3443

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
cheenamalhotra merged 3 commits intodotnet:mainfromapoorvdeshmukh:dev/genericVector
Jun 26, 2025

Conversation

apoorvdeshmukh
Copy link
Contributor

Description

This PR adds support for vector datatype with generic SqlType class SqlVector
After considering the feedback from discussion#3382, existing APIs are now supported with revised APIs from SqlVector
This helps in reducing the API surface area for unmanaged types.

Issues

#3317

Testing

Unit Tests for SqlVector
BackWardCompatibility Test suite to ensure support for APIs that worked with varchar(max) for vector datatype.
Tests for native vector datatype.

cheenamalhotra reacted with thumbs up emoji
@CopilotCopilotAI review requested due to automatic review settingsJune 25, 2025 17:37
@apoorvdeshmukhapoorvdeshmukh requested a review froma team as acode ownerJune 25, 2025 17:37
Copy link
Contributor

@CopilotCopilotAI 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

This PR replaces the legacySqlVectorFloat32 type with a new genericSqlVector<T> implementation, updating all client code, tests, and documentation to use the generic API.

  • IntroducesSqlVector<T> in place ofSqlVectorFloat32, handling only supported unmanaged types
  • RemovesSqlVectorFloat32 code, test classes, and related resource entries
  • Updates SQL client surface (SqlDataReader,SqlParameter,SqlBuffer,ISqlVector), tests, and XML docs to reference the generic API

Reviewed Changes

Copilot reviewed 22 out of 23 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlTypes/SqlVector.csAdds genericSqlVector<T> implementation
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlTypes/SqlVectorFloat32.csDeleted obsolete specific vector type
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDataReader.csReplacesGetSqlVectorFloat32 with genericGetSqlVector<T>
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.csUpdates vector payload coercion and return to use generic
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBuffer.csReplacesSqlVectorFloat32 uses with generic accessors
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ISqlVector.csUpdates interface to match generic API
src/Microsoft.Data.SqlClient/tests/UnitTests/SqlVectorTest.csAdds unit tests for generic vector behavior
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/VectorTest/NativeVectorFloat32Tests.csUpdates manual tests to useSqlVector<float>
src/Microsoft.Data.SqlClient/src/Resources/Strings.resxRemoves unused vector resource strings
doc/snippets/Microsoft.Data.SqlTypes/SqlVector.xmlAdds XML docs for genericSqlVector<T>
src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.csRemoves obsolete vector helpers
Files not reviewed (1)
  • src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs: Language not supported
Comments suppressed due to low confidence (2)

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/VectorTest/NativeVectorFloat32Tests.cs:100

  • [nitpick] The method nameValidateSqlVectorFloat32Object no longer matches the genericSqlVector<T> type; consider renaming it to something likeValidateSqlVectorObject for clarity.
        private void ValidateSqlVectorFloat32Object(bool isNull, SqlVector<float> sqlVectorFloat32, float[] expectedData, int expectedSize, int expectedLength)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ISqlVector.cs:28

  • [nitpick] Add an XML doc comment forVectorPayload to explain that it returns the raw TDS payload of the vector.
        byte[] VectorPayload { get; }

Copy link
Contributor

@paulmedynskipaulmedynski left a comment

Choose a reason for hiding this comment

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

Mostly commentary, and a few minor changes.

@apoorvdeshmukhapoorvdeshmukh mentioned this pull requestJun 25, 2025
3 tasks
paulmedynski
paulmedynski previously approved these changesJun 25, 2025
cheenamalhotra
cheenamalhotra previously approved these changesJun 25, 2025
@cheenamalhotracheenamalhotra added this to the6.1-preview2 milestoneJun 25, 2025
Copy link
Contributor

@mdaiglemdaigle left a comment

Choose a reason for hiding this comment

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

Looks good. I like the generic API.

apoorvdeshmukh reacted with heart emoji
mdaigle
mdaigle previously approved these changesJun 25, 2025
@codecovCodecov
Copy link

codecovbot commentedJun 26, 2025•
edited
Loading

Codecov Report

Attention: Patch coverage is79.00000% with21 lines in your changes missing coverage. Please review.

Project coverage is 58.84%. Comparing base(fc950e8) to head(6095de8).
Report is 1 commits behind head on main.

Files with missing linesPatch %Lines
...qlClient/src/Microsoft/Data/SqlClient/SqlBuffer.cs0.00%10 Missing⚠️
...ient/src/Microsoft/Data/SqlClient/SqlDataReader.cs14.28%6 Missing⚠️
...lient/src/Microsoft/Data/SqlClient/SqlParameter.cs25.00%3 Missing⚠️
...SqlClient/src/Microsoft/Data/SqlTypes/SqlVector.cs97.43%2 Missing⚠️

❗ There is a different number of reports uploaded between BASE (fc950e8) and HEAD (6095de8). Click for more details.

HEAD has 1 upload less than BASE
FlagBASE (fc950e8)HEAD (6095de8)
addons10
Additional details and impacted files
@@            Coverage Diff             @@##             main    #3443      +/-   ##==========================================- Coverage   64.55%   58.84%   -5.71%==========================================  Files         281      275       -6       Lines       62519    62220     -299     ==========================================- Hits        40358    36614    -3744- Misses      22161    25606    +3445
FlagCoverage Δ
addons?
netcore61.52% <78.72%> (-5.42%)⬇️
netfx62.34% <78.12%> (-5.61%)⬇️

Flags with carried forward coverage won't be shown.Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report?Share it here.

🚀 New features to boost your workflow:
  • ❄️Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cheenamalhotracheenamalhotra merged commit27fd1f6 intodotnet:mainJun 26, 2025
233 of 237 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

Copilot code reviewCopilotCopilot left review comments

@benrr101benrr101benrr101 approved these changes

@cheenamalhotracheenamalhotracheenamalhotra approved these changes

@paulmedynskipaulmedynskipaulmedynski approved these changes

@samsharma2700samsharma2700samsharma2700 approved these changes

@mdaiglemdaigleAwaiting requested review from mdaigle

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
6.1-preview2
Development

Successfully merging this pull request may close these issues.

6 participants
@apoorvdeshmukh@benrr101@mdaigle@cheenamalhotra@paulmedynski@samsharma2700
[8]ページ先頭
©2009-2025 Movatter.jp