Skip to content

Avoid redundant array alloc when TryReadSqlValueInternal for SQLVECTOR#4156

Merged
paulmedynski merged 2 commits intodotnet:mainfrom
SimonCropp:Avoid-redundant-array-alloc-when-TryReadSqlValueInternal-for-SQLVECTOR
Apr 28, 2026
Merged

Avoid redundant array alloc when TryReadSqlValueInternal for SQLVECTOR#4156
paulmedynski merged 2 commits intodotnet:mainfrom
SimonCropp:Avoid-redundant-array-alloc-when-TryReadSqlValueInternal-for-SQLVECTOR

Conversation

@SimonCropp
Copy link
Copy Markdown
Contributor

No description provided.

@github-project-automation github-project-automation Bot moved this to To triage in SqlClient Board Apr 8, 2026
@SimonCropp SimonCropp requested a review from a team as a code owner April 8, 2026 02:44
Copilot AI review requested due to automatic review settings April 8, 2026 02:44
Copy link
Copy Markdown
Contributor

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

Removes an unnecessary byte[] allocation when reading binary/vector TDS values in TryReadSqlValueInternal, relying on TryReadByteArrayWithContinue to allocate (or reuse snapshot storage) as needed.

Changes:

  • Eliminates a redundant new byte[length] prior to calling TryReadByteArrayWithContinue(...).
  • Uses inline out byte[] b declaration to keep allocation responsibility inside TryReadByteArrayWithContinue.

@paulmedynski paulmedynski self-assigned this Apr 10, 2026
@paulmedynski paulmedynski moved this from To triage to In review in SqlClient Board Apr 10, 2026
@paulmedynski paulmedynski added this to the 7.1.0-preview1 milestone Apr 10, 2026
@paulmedynski
Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 2 pipeline(s).

@rhuijben
Copy link
Copy Markdown

👍🏼

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.30%. Comparing base (62d96ad) to head (66ea905).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4156      +/-   ##
==========================================
- Coverage   66.02%   64.30%   -1.73%     
==========================================
  Files         277      272       -5     
  Lines       42989    65783   +22794     
==========================================
+ Hits        28383    42301   +13918     
- Misses      14606    23482    +8876     
Flag Coverage Δ
CI-SqlClient ?
PR-SqlClient-Project 64.30% <100.00%> (?)

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.

@paulmedynski paulmedynski enabled auto-merge (squash) April 24, 2026 11:35
@paulmedynski
Copy link
Copy Markdown
Contributor

@SimonCropp - The macOS failures are pretty weird. Can you rebase/merge from main? Lots of new build-related work there.

…ay-alloc-when-TryReadSqlValueInternal-for-SQLVECTOR
@SimonCropp
Copy link
Copy Markdown
Contributor Author

@paulmedynski done

@paulmedynski
Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 2 pipeline(s).

@paulmedynski paulmedynski merged commit 86324fd into dotnet:main Apr 28, 2026
303 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants