Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE REQUEST] Allow sending UUID in binary to database #1582

Closed
knutwannheden opened this issue May 18, 2021 · 5 comments · Fixed by #2324 or #2370
Closed

[FEATURE REQUEST] Allow sending UUID in binary to database #1582

knutwannheden opened this issue May 18, 2021 · 5 comments · Fixed by #2324 or #2370
Labels
Backlog The topic in question has been recognized and added to development backlog Enhancement An enhancement to the driver. Lower priority than bugs.

Comments

@knutwannheden
Copy link

Is your feature request related to a problem? If so, please give a short summary of the problem and how the feature would resolve it

I am trying to execute SQL queries with UUID objects in Java mapping to uniqueidentifier values in the database. This works, but when passing a UUID value as an input to a query using PreparedStatement#setObject() the value ends up being transferred as a string over the wire. This is inefficient both computationally (UUID#toString()) and also transfers more data over the network (36 bytes vs. 16 bytes).

Describe the preferred solution

When reading uniqueidentifier values from a ResultSet I can call ResultSet#getObject(int, Class) with the second parameter as UUID.class. This will give back the correct UUID value as expected without first converting the byte array to a String. Essentially I would want to have the same possibility when calling PreparedStatement#setObject() with a UUID object as input.

Describe alternatives you've considered

The alternative (workaround) I now use to do this is to manually convert the UUID value to a byte[] value (essentially using the same logic as in Util#asGuidByteArray(UUID)) and then give that byte[] object to PreparedStatement#setObject().

Additional context

Looking at the code I found this relevant block:

if (JDBCType.GUID == jdbcType) {
if (null != cryptoMeta) {
if (value instanceof String) {
value = UUID.fromString((String) value);
}
byte[] bArray = Util.asGuidByteArray((UUID) value);
op.execute(this, bArray);
} else {
op.execute(this, String.valueOf(value));
}

Apparently it looks like what I want is possible, but only when cryptoMeta is set. I don't understand how encryption is related to this.

Reference Documentations/Specifications

Reference Implementation

@knutwannheden knutwannheden added the Enhancement An enhancement to the driver. Lower priority than bugs. label May 18, 2021
@VeryVerySpicy
Copy link
Contributor

Hi Knutwannheden,

This looks like a possible improvement to add in the future. It looks like this separation in flow was deliberately put in place, so there is risk of breaking something if we are not careful. Regardless it has been backlogged.

@Jeffery-Wasty Jeffery-Wasty added the Backlog The topic in question has been recognized and added to development backlog label Mar 10, 2022
@vxel
Copy link
Contributor

vxel commented Feb 8, 2024

Looking at the code, it seems that there is currently no way to pass a UUID as a TDSType.GUID (0x24) on the wire.
When you pass the parameter as byte[], the TDS type seems to be TDSType.BIGVARBINARY (0x45).
When you pass it as String, the TDS type is TDSType.NVARCHAR (0xE7).

I may contribute a PR for this feature.

vxel pushed a commit to vxel/mssql-jdbc that referenced this issue Feb 8, 2024
@lilgreenbird lilgreenbird linked a pull request Feb 14, 2024 that will close this issue
lilgreenbird pushed a commit that referenced this issue Mar 20, 2024
* Added support for TDSType.GUID (#1582)

* Fix UUID serialisation

* Add unit tests

* Fix typo, handle PR feedback

---------

Co-authored-by: Cédric de Launois <[email protected]>
Co-authored-by: Cédric de Launois <>
tkyc added a commit that referenced this issue Mar 25, 2024
lilgreenbird pushed a commit that referenced this issue Mar 25, 2024
vxel pushed a commit to vxel/mssql-jdbc that referenced this issue Mar 27, 2024
@lilgreenbird lilgreenbird reopened this Mar 27, 2024
@lilgreenbird
Copy link
Contributor

lilgreenbird commented Mar 27, 2024

re-opening since PR #2324 was reverted

@knutwannheden
Copy link
Author

Thanks for fixing this! I do have a small comment, however: To me it looks like the implementation currently requires the UUID to first be converted to a String only to then convert it back to a UUID again. It was in part also these seemingly unnecessary object allocations which led me to report this issue. Is there no way for the UUID to be provided as-is via setObject()?

@vxel
Copy link
Contributor

vxel commented Apr 3, 2024

Indeed PR #2370 is required but does not fully cover this feature request.
I would also like to be able to directly pass a UUID to the setObject but I'm not fully confident in knowing all the impacts of such change.
I may try to dive into this if a core maintainer confirms this is the way to go and/or provides some guidance.

vxel pushed a commit to vxel/mssql-jdbc that referenced this issue May 10, 2024
@github-project-automation github-project-automation bot moved this to Closed Issues in MSSQL JDBC Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backlog The topic in question has been recognized and added to development backlog Enhancement An enhancement to the driver. Lower priority than bugs.
Projects
Status: Closed Issues
5 participants