Feature | Send uniqueidentifier columns natively in bulk copy

[Siarhei (huzaus)]

Brief Description

Bulk copy declared GUID columns as CHAR(36) on the wire, so the server converted every single row on the way into a uniqueidentifier column. This sends the native type instead.

What changed in SQLServerBulkCopy:

• getDestTypeFromSrcType now returns uniqueidentifier rather than CHAR(n)
• writeTypeInfo emits the TDSType.GUID token. It could only be reached via the isBaseType branch before, which is written for Always Encrypted base type metadata only, so normal columns always fell through to BIGCHAR
• values go out as the 16 mixed endian bytes from Util.asGuidByteArray, same as writeRPCUUID; null is a 0x00 length byte
• a value that is not a GUID is reported as a SQLServerException using R_errorConvertingValue, chaining the IllegalArgumentException from UUID.fromString, so nothing unchecked escapes writeToServer in the middle of a batch

Only applies when the source declares microsoft.sql.Types.GUID and the destination is uniqueidentifier. CHAR/VARCHAR sources and the Always Encrypted path are untouched.

There is no connection property. Following the review, the registry format in braces is normalized rather than rejected, so this stays a wire format change.

Fixes existing GitHub issue

Fixes #2986

New Public APIs

None.

Behavior

A source column of type java.sql.Types.CHAR with the same precision reproduces the previous character wire format exactly, which makes the old and the new behavior comparable inside one build. Measured on SQL Server 2022, per rendering of the same GUID:

Rendering	Character format	Native format
6f9619ff-...-00c04fc964ff	stored	stored
6F9619FF-...-00C04FC964FF	stored	stored
{6f9619ff-...-00c04fc964ff} declared 38	stored	stored
{6f9619ff-...-00c04fc964ff} declared 36	rejected	stored
6f9619ff-...-00c04fc964ff	rejected	rejected
6f9619ff8b86d011b42d00c04fc964ff	rejected	rejected
(...), urn:uuid:..., not-a-guid, empty	rejected	rejected

The one difference is the row in bold above: the character format truncated the value to the declared precision before the server converted it, so the registry format did not fit a column declared with 36 characters. Parsing on the client no longer depends on the declared precision. Braces are accepted at every precision, whitespace is rejected as before, and rejections that used to come from the server now come from the driver.

Verification

New BulkCopyGuidTest, 29 cases, run against SQL Server 2022 (16.0.4265.3):

• testBulkCopyGuidNativeFormatKeepsCharacterFormatBehavior — every rendering above through both wire formats, asserting equal outcomes
• testBulkCopyGuidNativeFormatIgnoresDeclaredPrecision — pins the single deliberate difference
• testBulkCopyGuidUnparsableValueFailsOnClient — 7 unparsable renderings, asserting the exact driver message; testBulkCopyGuidByteArrayValueFailsOnClient covers a byte[] value. These run everywhere, including Azure SQL Database, and are the deterministic check that the native format is in use, since the character format would have failed on the server instead
• testBulkCopyGuidConversionOnServer — Extended Events on plan_affecting_convert, parameterized over both source types: nothing is reported against [!BulkInsert] for a GUID source, and a conversion is reported for a CHAR source. The CHAR case is the positive control, so the GUID case cannot pass by capturing nothing at all. A conversion issued by the test acts as a dispatch barrier, and the session is closed through AutoCloseable, so a failed STATE = START cannot leave a server scoped session behind. The whole test is skipped unless HAS_PERMS_BY_NAME reports both permissions on a server that is not Azure SQL Database
• testBulkCopyGuidIntoCharacterDestination — a GUID source into char, varchar, nchar and nvarchar destinations still goes out as a character string
• testBulkCopyGuidRoundTripsValues — UUID, lowercase string and null through ISQLServerBulkData

New BulkCopyGuidAETest, run against a local Java key store column master key:

• testBulkCopyGuidIntoEncryptedColumn — an encrypted uniqueidentifier column still receives ciphertext, verified by reading the column over a connection without column encryption
• testBulkCopyGuidSourceIntoEncryptedColumnIsRejected — pins that a GUID source into an encrypted column is rejected, which is also the behavior on main, since validateDataTypeConversions checks against the base type of the encrypted column

Against pristine main, testBulkCopyGuidConversionOnServer fails for the GUID source and passes for the CHAR source, reporting:

INSERT BULK [guidConversionDest_...] ([id] CHAR(36) )
CONVERT_IMPLICIT(uniqueidentifier,[!BulkInsert].[id],0)

After the change the GUID source reports no conversion.

Full bulk copy suite: 113 tests, the only failure being the pre-existing temporal flake in BulkCopyColumnMappingTest, which passes on a rerun with unchanged code. A GUID source into a varchar(max) destination fails identically on main and is left alone.

[Siarhei (huzaus)]

@microsoft-github-policy-service agree

[Muskan Gupta (muskan124947)]

Siarhei (@huzaus)
I don’t think we need to introduce a new connection string option for this change. The existing behavior/configuration already covers the intended scenario, and adding another option would unnecessarily expand the public configuration surface and increase maintenance and documentation overhead. Could we implement this using the existing behavior instead?

[Siarhei (huzaus)]

Muskan Gupta (@muskan124947) Property removed, uniqueidentifier is now always sent natively. Ready for review.

[Muskan Gupta (muskan124947)]

Siarhei (@huzaus)
Adding below comments, please have a look:

1. writeGuidToTdsWriter — wrap the parse failure in SQLServerException
UUID.fromString(...) throws an unchecked IllegalArgumentException, which escapes writeToServer() mid-batch. The driver should only surface SQLServerException. Please wrap it using R_errorConvertingValue, following the pattern in SQLServerBulkCSVFileRecord.getRowData() for BIGINT. Same applies when colValue is a byte[] — toString() yields "[B@..." and hits the same path.

2. Behavior change needs to be non-breaking
   Values the server previously accepted, e.g. {0E984725-...}, now fail client-side, and with the property removed there is no escape hatch. To answer your open question: please normalize rather than reject — strip braces before UUID.fromString — so this is a pure wire-format optimization with zero user-visible behavior change. That keeps the connection property unnecessary.

3. testBulkCopyGuidDoesNotConvertOnServer needs a positive control
   The test asserts an absence, so it passes vacuously if the XE harness misbehaves. The control was deleted along with the property. Please add one: a java.sql.Types.CHAR source into the same uniqueidentifier destination, asserting the conversion is captured.

4. Test cleanup issues

• If CREATE EVENT SESSION succeeds but ALTER ... STATE = START fails, canCaptureImplicitConversions returns false, the assumption aborts, and dropEventSession never runs — orphaned server-scoped session.
• In the finally, if dropEventSession throws, dropTableIfExists is skipped. Please nest them.
• catch (SQLException e) { return false; } hides real failures.
• Server-scoped XE isn't available on Azure SQL DB, so this guard silently skips in most CI legs. Consider adding a deterministic assertion on getDestTypeFromSrcType that always runs.

5. Missing test cases
   Invalid GUID error path, {braces} input, CHAR source → uniqueidentifier dest (no-regression), and an AE-encrypted uniqueidentifier column — the writeTypeInfo condition being modified is the AE one, so it should have coverage.

Also, please update the PR description

[Siarhei (huzaus)]

Muskan Gupta (@muskan124947) All five done in ec5724dd.

1. Fixed. Wrapped in SQLServerException with R_errorConvertingValue, chaining the IllegalArgumentException. Same for a byte[] value.

2. Fixed. Control added back. The test now runs twice: GUID source, expecting no conversion, and CHAR source, expecting one. The CHAR run confirms the session really captures events, so an empty result in the GUID run means something. Checked on main: GUID run fails, CHAR run passes.

3. Fixed. Session is AutoCloseable and drops itself if STATE = START fails, table drop is nested in its own finally, and the swallowed catch is replaced by a HAS_PERMS_BY_NAME plus engine edition guard. For the check that always runs, including on Azure SQL Database, I used the error path instead of the private getDestTypeFromSrcType: an unparsable value fails on the client, where the old format failed on the server.

4. Fixed. Invalid GUID, braces, CHAR into uniqueidentifier, and an AE encrypted column are covered, plus GUID into char/varchar/nchar/nvarchar.

5. Braces are stripped now, as asked. One correction though: braces were not accepted before either, in the usual case. I compared the two formats by declaring the source as java.sql.Types.CHAR with the same precision, which reproduces the old wire format. It truncates the value to the declared precision before the server converts, so {0E984725-...} needs the column declared with 38, and was rejected at the usual 36. Full table is in the PR description. The two formats now agree everywhere except braces at precision 36, which is accepted now because client side parsing ignores the declared precision. Pinned by a test, noted in the CHANGELOG.

Same comparison showed whitespace was always rejected, so the trim() I had would have quietly accepted more than before. Removed.

On AE: a GUID source into an encrypted column is rejected on main too, because validateDataTypeConversions checks against the base type. So the case that can be tested is a CHAR source, and testBulkCopyGuidIntoEncryptedColumn confirms over a non encrypting connection that the column really holds ciphertext. The rejection is pinned separately.

Tests moved to BulkCopyGuidTest, so BulkCopyAllTypesTest is back to its main state. One thing I noticed and left alone, since it behaves the same on main: a GUID source into a varchar(max) destination fails with "Invalid column type from bcp client for colid 1". PR description updated.

[Muskan Gupta (muskan124947)]

Thanks Siarhei (@huzaus) , will take a look