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

Update MS SQL Server JDBC driver #13948

Merged
merged 1 commit into from
Nov 23, 2022
Merged

Conversation

mosabua
Copy link
Member

@mosabua mosabua commented Sep 1, 2022

Description

Update the JDBC driver for the SQL Server connector to the new major release (11) and also use the Java 17 version instead of the Java 11 version. The new driver is fully 4.2. JDBC spec compliant among other improvements.

Is this change a fix, improvement, new feature, refactoring, or other?

Improvement.

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

SQL Server connector only.

How would you describe this change to a non-technical end user or system administrator?

Update to new JDBC driver mostly for maintenance reasons.

Related issues, pull requests, and links

Documentation

( ) No documentation is needed.
(x) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

( ) No release notes entries required.
(x) Release notes entries required with the following suggested text:

## SQL Server connector
* to be determined ({issue}`13948`)

There are a bunch of fixes and improvements in the JDBC driver that could affect Trino usage but I am not sure there are. Some of the things supported by the driver like IPv6 config and such can potentially be used with connection string parameters but I am uncertain what to mention in the release notes for Trino.

E.g. you can now enable TDS 8.0 support when the property encrypt is set to strict, which in fact is a breaking change for users that used that property before.

@cla-bot cla-bot bot added the cla-signed label Sep 1, 2022
@mosabua mosabua changed the title Update MS SQL Server JDBC driver WIP: Update MS SQL Server JDBC driver Sep 1, 2022
@mosabua
Copy link
Member Author

mosabua commented Sep 1, 2022

I filed this as WIP for now to verify passing test suite while checking for other necessary updates.

pom.xml Outdated Show resolved Hide resolved
@mosabua
Copy link
Member Author

mosabua commented Nov 17, 2022

Once CI passes this is ready for merge imho .. however I will need help to determine if a RN entry is needed, and if yes.. what it should say.

@mosabua mosabua changed the title WIP: Update MS SQL Server JDBC driver Update MS SQL Server JDBC driver Nov 17, 2022
@hashhar
Copy link
Member

hashhar commented Nov 18, 2022

For RN I don't anything user-facing. So we can either not mention it or just mention driver version is updated. What did we do in past?

@mosabua
Copy link
Member Author

mosabua commented Nov 18, 2022

@hashhar last time there were using facing breakages so we documented. In this case there are as well .. but super edge usage.. so I think we can get away without entry

@mosabua
Copy link
Member Author

mosabua commented Nov 22, 2022

@ebyhr @hashhar @martint .. merge maybe?

@hashhar
Copy link
Member

hashhar commented Nov 23, 2022

There is:

TDS 8.0 support by adding "strict" option to encrypt connection property | BREAKING CHANGE Encrypt connection property is now of type string.

But it's not breaking from Trino's perspective since we don't set it. However it would be good idea to mention in release notes that the driver version is updated (and link to release notes) so people can do any changes that they need for their setup.

@hashhar hashhar merged commit 36fea75 into trinodb:master Nov 23, 2022
@github-actions github-actions bot added this to the 404 milestone Nov 23, 2022
@mosabua mosabua deleted the sqlserver-jdbc branch November 23, 2022 19:42
@mosabua
Copy link
Member Author

mosabua commented Nov 23, 2022

Thanks for the merge @hashhar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants