Skip to content

Conversation

apoorvdeshmukh
Copy link
Contributor

Description

  • Removed Size property since it can be inferred from SqlParameter.Value. Same as for output parameters.
    In case specified, it will be ignored.
  • Changed ctor1(length) to CreateNull(length) to make it clearer that this method creats object representing null instance of given length.
  • Changed SqlVector from sealed class to readonly struct.
  • Test modifications to accommodate above changes.

Issues

Incorporating suggestions from #3382

Testing

Modified existing testcases for above changes.

Guidelines

Please review the contribution guidelines before submitting a pull request:

@Copilot Copilot AI review requested due to automatic review settings July 9, 2025 18:37
@apoorvdeshmukh apoorvdeshmukh requested a review from a team as a code owner July 9, 2025 18:37
Copy link
Contributor

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

This PR enhances the vector datatype API by replacing the length‐based constructor with a clearer factory, removing the public Size property (now inferred), converting SqlVector<T> to a readonly struct, and updating the SQL parameter handling and tests to match.

  • Refactored SqlVector<T>: made it a readonly struct, dropped the public Size property, and added CreateNull(int length).
  • Updated SqlParameter to ignore Size for vector types and adjusted Prepare logic.
  • Revised unit and manual tests to use CreateNull, removed direct Size assertions, and updated documentation and ref surfaces.

Reviewed Changes

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

Show a summary per file
File Description
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlTypes/SqlVector.cs Converted SqlVector<T> to struct, added CreateNull, removed Size property
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs Skip applying Size for vector parameters and update Prepare check
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDataReader.cs Simplified vector‐to‐string conversion logic
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBuffer.cs Use CreateNull when returning a null vector
src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlTypes/SqlVectorTest.cs Updated tests to use CreateNull and check Size via ISqlVector
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/VectorTest/NativeVectorFloat32Tests.cs Removed sizeInbytes, adapted test data and validation
src/Microsoft.Data.SqlClient/net*/ref/Microsoft.Data.SqlClient.cs Updated API surface: SqlVector<T> now readonly struct, added CreateNull
doc/snippets/Microsoft.Data.SqlClient/SqlCommand.xml Clarified in docs that Size is ignored for vectors

Copy link

codecov bot commented Jul 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (8eb9f32) to head (8ec2ca7).
Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #3468       +/-   ##
==========================================
- Coverage   68.86%       0   -68.87%     
==========================================
  Files         280       0      -280     
  Lines       62417       0    -62417     
==========================================
- Hits        42982       0    -42982     
+ Misses      19435       0    -19435     
Flag Coverage Δ
addons ?
netcore ?
netfx ?

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.

Copy link
Contributor

@paulmedynski paulmedynski left a comment

Choose a reason for hiding this comment

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

A few minor comments.

@apoorvdeshmukh apoorvdeshmukh added this to the 6.1.0 milestone Jul 10, 2025
@apoorvdeshmukh apoorvdeshmukh added the Area\Vector Use this for issues that are targeted for the Vector feature in the driver. label Jul 10, 2025
@cheenamalhotra cheenamalhotra merged commit 2bb2c43 into dotnet:main Jul 10, 2025
237 checks passed
@apoorvdeshmukh apoorvdeshmukh self-assigned this Jul 11, 2025
apoorvdeshmukh added a commit that referenced this pull request Jul 11, 2025
@paulmedynski paulmedynski removed this from the 6.1.0 milestone Jul 11, 2025
apoorvdeshmukh added a commit that referenced this pull request Jul 11, 2025
apoorvdeshmukh added a commit that referenced this pull request Jul 12, 2025
This commit ports #3468 from main to release/6.1
@David-Engel David-Engel changed the title API enahncements for vector datatype API enhancements for vector datatype Jul 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area\Vector Use this for issues that are targeted for the Vector feature in the driver.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants