Feature | Send uniqueidentifier columns natively in bulk copy

[Muskan Gupta (muskan124947)]

Note

Community PR #3006 by Siarhei (@huzaus)

Source: huzaus/mssql-jdbc:feature/native-uniqueidentifier-bulkcopy at ec5724ddcb1902ca1f22f7c89e1fd08eedac8bc7

Brief Description

Send eligible bulk-copy GUID columns using native TDS uniqueidentifier encoding rather than CHAR(n), avoiding server-side character-to-GUID conversion. GUID values were already supported; this changes their transfer representation while preserving the tested input acceptance and declared-precision constraints.

In SQLServerBulkCopy:

• A shared boolean isNativeGuid(source, destination) check selects native encoding consistently for the bulk SQL declaration, column metadata, and row payload.
• Existing source JDBC type assignments, source metadata caching, getSourceMetadata, writeTypeInfo, and writeColumnToTdsWriter retain their original implementation. The native case is handled by narrow branches around the existing paths.
• No source ssType storage/copying or new srcColumnMetadata = null resets are needed. For SQL Server ResultSets the check inspects the current column's native type, without changing cached/public JDBC metadata.
• For an active ISQLServerBulkData source, including custom records and CSV, eligibility reads the current source's getColumnType() rather than its cached type. This handles GUID-to-CHAR and CHAR-to-GUID source reuse without resetting the general metadata cache. An active ResultSet takes precedence over any retained bulk-data object.
• Row eligibility is checked before reading source data because inspecting a ResultSet column can close an active stream. Streamed character sources retain their existing transfer path.
• Native values use a one-byte length of 16 followed by SQL Server GUID bytes from Util.asGuidByteArray; null uses a zero length byte.
• String parsing validates ASCII hexadecimal 8-4-4-4-12 layout before calling UUID.fromString, while preserving SQL Server's accepted braces/suffixes and legacy precision constraints.
• UUID and string inputs share length/precision validation without converting UUID objects to text.
• Invalid values raise a driver SQLServerException through R_errorConvertingValue, retaining the parsing cause. Invalid metadata is rejected before row transmission.

Path selection

Source / operation	Plaintext uniqueidentifier destination
Custom bulk record explicitly declaring microsoft.sql.Types.GUID	Native GUID encoding
CSV field explicitly declaring GUID	Native GUID encoding; CSV supplies string/null values
Plaintext GUID column from SQLServerResultSet	Native GUID encoding, using current internal SQL Server metadata locally within bulk copy
Eligible executeBatch() / executeLargeBatch() with useBulkCopyForBatchInsert=true	Native GUID encoding; the adapter derives GUID metadata from the destination, including for setString() input
CHAR/VARCHAR source, including streamed VARCHAR(MAX)	Existing character transfer and server-side conversion
Other ResultSet implementations / RowSets	Existing declared JDBC metadata; no inference from GUID-looking text

GUID-to-character destinations and encrypted sources/destinations retain their existing paths. Encrypted-value-modification handling is not expanded. No connection property is introduced.

For SQL Server ResultSets, public ResultSetMetaData.getColumnType() remains unchanged: GUID is still exposed as JDBC CHAR. Native selection is per destination, allowing the same source column to map to both GUID and character destinations. Current-source eligibility avoids retaining a stale native GUID decision when the bulk-copy object is reused. This is a narrow eligibility fix, not a general source-schema cache refresh: cached precision, scale, names, and other metadata retain their existing behavior.

Fixes existing GitHub issue

Fixes #2986

New Public APIs

None. Updated writeToServer(ResultSet) Javadoc describes native selection and retained fallbacks. Getter/setter/updater conversion mappings are unchanged.

Compatibility behavior

Regression tests compare the native GUID path with the legacy character path and assert expected stored values or rejection, not merely agreement. The previous GUID character-wire path was also temporarily restored during the earlier investigation to confirm precision behavior.

Here, GUID means a complete 36-character canonical GUID:

Input	Legacy character path and corrected native path
Lowercase or uppercase GUID	Accepted when the full input fits the declared precision
{GUID}	Accepted at precision 38 or greater; rejected at precision 36
GUID followed by text, spaces, or tabs	Accepted when the full input fits; suffix does not change the stored GUID
{GUID} followed by text or spaces	Accepted when the full input fits; closing brace must immediately follow the GUID
Leading whitespace, short/wrong-length groups, signs, invalid hex, missing separators, malformed braces	Rejected
Input longer than declared precision	Rejected, not truncated to that precision
UUID object	Accepted at precision 36 or greater without converting the object to text
SQL NULL	Preserved with valid source metadata

The former CHAR(n) metadata range of 1 through 8000 remains enforced, including null rows. Full input must fit before suffix processing. This is a legacy compatibility restriction, not the native GUID wire size. Exact length alone is insufficient: Java accepts some malformed 36-character inputs, so the parser's format checks remain necessary.

Outstanding transaction compatibility decision

Invalid inputs that previously failed on the server can now fail on the client. The reported caller-owned transaction regression remains open: this can change whether earlier work is committed by a subsequent commit(). See the original reproduction and the outstanding review comment.

This PR currently defers this policy decision. It does not introduce an opt-in, force rollback of caller-owned transactions, or claim complete failure-semantics compatibility. Native conversion failures remain driver SQLServerExceptions and surface as BatchUpdateException through the batch APIs.

CSV bulk load

SQLServerBulkCSVFileRecord supplies strings. Declaring addColumnMetadata(..., microsoft.sql.Types.GUID, precision, 0) selects native GUID transfer into a plaintext uniqueidentifier destination. Declaring CHAR retains character transfer. Empty CSV fields remain SQL NULL, unlike an empty String from a custom record.

CSV round-trip/compatibility tests do not require Extended Events permissions and are not excluded from Azure SQL Database.

Verification

Before rebasing, combined validation passed under both jre11 and jre8 Maven profiles on JDK 21: 449 tests per profile, zero failures/errors/skips. After rebasing onto main and removing the explanatory Markdown document, the same combined suite passed again under jre11. The jre8 profile was not rerun after the rebase. These runs are not execution on an actual JDK 8 runtime or a full repository suite pass.

mvn -q -Pjre11 -Dtest=BulkCopyGuidParserTest,BulkCopyGuidMetadataTest,BulkCopyGuidTest,BulkCopyGuidBatchInsertTest,BulkCopyAllTypesTest test
mvn -q -Pjre8 -Dtest=BulkCopyGuidParserTest,BulkCopyGuidMetadataTest,BulkCopyGuidTest,BulkCopyGuidBatchInsertTest,BulkCopyAllTypesTest test

Coverage:

• BulkCopyGuidParserTest: 33 unit cases, including exactly-36-character malformed group lengths.
• BulkCopyGuidMetadataTest: 11 cases for per-destination eligibility, current ResultSet metadata, conflicting cached/current bulk-data types, active ResultSet precedence over retained bulk data, fallback guards, exact mixed-endian payload bytes, and null encoding.
• BulkCopyGuidTest: 375 integration cases covering custom-record/CSV compatibility; ResultSet GUID/CHAR/VARCHAR/VARCHAR(MAX) sources into GUID and character destinations; forward-only/scroll-insensitive cursors; first-row nulls; mixed columns; reordered/duplicate mappings; batch boundaries; source reuse; native command declarations and server-side conversion checks. Custom-record and CSV reuse is covered in both GUID-to-CHAR and CHAR-to-GUID order, asserting declarations, stored values, and malformed-input error origin.
• BulkCopyGuidBatchInsertTest: 26 cases covering both APIs, bulk/non-bulk successful-value parity, string/UUID/GUID/null bindings, mixed columns, statement reuse, bulk options, malformed values, and bulk precision failures.
• BulkCopyAllTypesTest: 4 existing cases.

Native declaration assertions run without Extended Events permissions. Server conversion assertions use character sources as positive controls and require suitable server permissions. They ran without skips in the pre-rebase validation.

Before implementation, the native ResultSet conversion test failed while its character controls passed. The six added custom-record/CSV source-reuse regression cases also failed before the current-bulk-data eligibility fix and passed afterward. These fixes preserve general source metadata caching without resets. Changed Java was formatted with the repository Eclipse profile.

Remaining validation limits

• BulkCopyGuidAETest was attempted during the core follow-up, but shared AE setup fails because target/test-classes/JavaKeyStore.txt is missing. Existing encrypted-source/destination coverage is retained; no AE execution pass is claimed.
• Reordered INSERT columns are covered for executeBatch; executeLargeBatch uses destination order because its existing reordered-column mapping limitation is outside this change.
• Transaction compatibility remains deferred; successful-value tests do not resolve it.
• No quantitative baseline/head benchmark was run. Native encoding and removal of the targeted server conversion are verified, but end-to-end throughput/CPU improvement is not quantified.

[Copilot]

🔵 Needs a closer look

It changes low-level bulk-copy TDS type/value encoding for GUIDs, so it warrants final human review despite strong test coverage.

Pull request overview

This PR updates SQLServerBulkCopy to send uniqueidentifier values using the native TDS GUID type (16 bytes) when the source declares microsoft.sql.Types.GUID and the destination is uniqueidentifier, avoiding per-row server-side implicit conversions and their performance/cardi­nality-estimation impact.

Changes:

• Emit TDSType.GUID type info and write GUID values as native 16-byte payloads for GUID→uniqueidentifier bulk copy.
• Adjust INSERT BULK column type declaration to uniqueidentifier for GUID sources targeting GUID destinations.
• Add comprehensive bulk-copy tests for plaintext and Always Encrypted GUID destinations, plus a CHANGELOG entry.

File summaries

File	Description
src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java	Switch GUID bulk-copy wire format to native GUID type/value encoding when destination is uniqueidentifier.
src/test/java/com/microsoft/sqlserver/jdbc/bulkCopy/BulkCopyGuidTest.java	Adds plaintext bulk-copy tests covering behavior parity, client-side parse failures, and XE conversion detection.
src/test/java/com/microsoft/sqlserver/jdbc/AlwaysEncrypted/BulkCopyGuidAETest.java	Adds AE-specific coverage to ensure encrypted uniqueidentifier remains ciphertext-based and GUID source rejection stays pinned.
CHANGELOG.md	Documents the behavior/performance change for GUID bulk copy into uniqueidentifier.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov]

Codecov Report

❌ Patch coverage is 94.33962% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.63%. Comparing base (ae8f44d) to head (3e4019e).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...om/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java	94.33%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #3041      +/-   ##
============================================
+ Coverage     61.50%   61.63%   +0.13%     
- Complexity     5288     5359      +71     
============================================
  Files           154      154              
  Lines         36817    36869      +52     
  Branches       6770     6789      +19     
============================================
+ Hits          22644    22725      +81     
+ Misses        10310    10308       -2     
+ Partials       3863     3836      -27     

☔ View full report in Codecov by Harness.
📢 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.

[Copilot]

Copilot review overview

🟢 Approval recommended

Only a non-blocking test assertion nit remains.

Review effort: Lite
Findings: None

Resolved since last review (1)

• The CSV header written by guidFileRecord() is "guidcol", but the bulk metadata (addColumnMetadata)…

[Muskan Gupta (muskan124947)]

Reviewed the native GUID bulk-copy change against merge-base 347d7755cdd46a1c01e3b23b02c707986264424d, including the parsing/precision and encrypted-source follow-up fixes. One new compatibility finding is attached: a reproduced change in the committed outcome of a caller-owned transaction after a malformed GUID, beyond the previously discussed parsing and exception changes.

Validation included the standalone parser suite, 311 non-Extended-Events integration cases, and identical baseline/head transaction probes on Windows/JDK 21 with SQL Server 16.0.1200.5. Always Encrypted execution, other runtime/platform combinations, and a quantitative performance comparison were not rerun; source review is not proof of compatibility or performance success.

[Muskan Gupta (muskan124947)]

Automated review — generated by GitHub Copilot on behalf of Muskan Gupta (@muskan124947), from an unattended run. These findings were not checked by a human before posting. This is not an approval and does not satisfy the human review requirement. Findings may be incomplete or incorrect; please challenge anything that looks off.

Summary

Follow-up review of 9e2e97a38f18827a40cc49fd23b9ab254c7b8b52 against previously reviewed ab84634ac882e1375b22c5c4d10e9b96202ef1f5: the new commit changes no files. The native GUID bulk-copy implementation and tests are identical; this empty-commit review does not clear or re-file existing findings.

Verification

Independently checked the REST commit/compare results: both revisions have tree 8ce550cba494485519ed93971ca1fb7259929e7c. Against current main tip a911b931a7c542a0fff08a8054fe591ea17659f9, the full-fix merge-base remains 347d7755cdd46a1c01e3b23b02c707986264424d. Read the PR discussion, prior reviews/replies and linked proposal; checked the unchanged driver diff for context. Scope is the empty incremental delta, not a fresh full-fix compatibility review. No tests or benchmarks were executed in this sweep; prior execution reports are not independent verification by this run.

Findings

Severity	Count
Blocking	0
Suggestion	0

No new high-confidence findings in the reviewed scope; this is not proof of compatibility.

Performance / CI status

Current checks are successful. Windows build 177988 retains an initial test-error summary, but its actual JDK 17 AzureDB task log records BUILD FAILURE, an automatic retry, then BUILD SUCCESS; this is not a currently failing check. That build used merge revision c878b3b7bd131be7ff9b73bd9ea95f21a3230cd2, whose parents are the pinned main tip and reviewed head. No comparable numeric baseline/head benchmark was found in the reviewed PR/proposal discussion or check evidence; performance remains unverified.

[Copilot]

Copilot review overview

🟡 Changes recommended

Unresolved transaction failure semantics and stale source-metadata handling can cause compatibility and correctness issues.

Review effort: Lite
Findings: 1 · 1

Open (2)

• Client parse failures can commit prior work in caller transactions · New
• Reset cached metadata when assigning a new bulk-data source · New

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 1 · 2

Open (3)

• The returned SQLServerBulkCSVFileRecord is constructed over an InputStream that is immediately… · New
• The GUID byte length is hard-coded as 0x10 in multiple places. Consider introducing a single… · New
• The GUID byte length is hard-coded as 0x10 in multiple places. Consider introducing a single… · New

Resolved since last review (2)

• Client parse failures can commit prior work in caller transactions
• Reset cached metadata when assigning a new bulk-data source