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

mssql-jdbc doesn't respect parameters that have stored procedures with default arguments defined #2446

Closed
brentryan opened this issue Jun 11, 2024 · 6 comments · Fixed by #2452
Labels
Bug A bug in the driver. A high priority item that one can expect to be addressed quickly.
Milestone

Comments

@brentryan
Copy link

Driver version

12.6.2

We know that it works properly with version 8.2.1.jre11 of the driver but this broke at some point after this version. We believe that it broke sometime after version 12.4.2.jre11 and probably in this diff: v12.5.0...v12.4.2

SQL Server version

We’re using SQL Server 2019 and later

Client Operating System

Amazon Linux 2, MacOS M2

JAVA/JVM version

Java 11, Java 17 and Java 21

Table schema

See attached test case for full example scenario.

Problem description

When calling a stored procedure that has default arguments specified, the value of the parameter (if passed in) gets ignored entirely.

See test case created DefaultMigrationsJunit5Test.java. You can execute this test using 8.2.1.jre11 and you will see the test pass. If you then update it to 12.6.2.jre11 then you’ll see the test case fail.

Expected behavior

The stored procedure should execute using the parameter that was passed in. The value that was passed into the parameter should override the default argument defined by the stored procedure.

Line 82 of DefaultMigrationsJunit5Test.java should have null results because the 3rd argument to the stored procedure was passed in with the value of 1.

Actual behavior

The stored procedure executes using only the default that was specified by the stored procedure. This breaks all kinds of custom logic where customers are using default arguments with conditional logic inside their stored procedures.

Line 82 of DefaultMigrationsJunit5Test.java is not null and has results because the stored procedure executed using the default defined inside the stored procedure.

How to reproduce

See mssql-jdbc-bug.zip file for full java project with unit test that reproduces this problem. Unzip project and utilize maven to run unit tests.

Note: This does require maven, java and docker desktop installed in order to successfully execute the tests.

mssql-jdbc-bug.zip

To see failing test run:

mvn test

To see passing test update the pom.xml file for the mssql-jdbc dependency to 8.2.1.jre11 and run:

mvn test

Remarks

We found this while preparing to upgrade our infrastructure to SQL Server 2022+ which led us to updating our versions of JDBC driver used across Cvent so that we stay within a supported version. We originally released code that was using 12.6.2 but this lead to several failures across our codebase.

@brentryan brentryan changed the title mssql-jdbc doesn't respect parameters that have stored procedures defined with default arguments defined mssql-jdbc doesn't respect parameters that have stored procedures with default arguments defined Jun 11, 2024
@tkyc
Copy link
Member

tkyc commented Jun 11, 2024

Thanks taking a look..

This likely broke because of the switch to execute stored procedures directly.

@tkyc
Copy link
Member

tkyc commented Jun 14, 2024

As a temporary workaround, you can rewrite your sql string sproc as:

exec dbo.getContactMaybe ?, ?, 1

This will get the 12.6.2 driver to respect the default argument.

@brentryan
Copy link
Author

That really doesn't work for us because we're talking about an entire enterprise with millions of lines of code, multiple teams, etc. They have relied on the previous behavior for years. I'm guessing that the rest of the community would be running into this as well.

I appreciate the work around but this would be considered a breaking change that should be documented and a major version bump of the driver. We are more likely to stay on old versions of the driver that we know works well.

Also, this behavior works fine with other drivers and other databases so my guess is that it's correct based on the JDBC specification.

We would rather have a fix put in place by Microsoft for this issue. And it would be great to understand when we might expect a fix for this so that we can plan accordingly.

It would also be great to add a unit test suite like this to the driver to capture regressions like this in the future.

Let me know if you need any help or feedback in accomplishing these goals.

@tkyc
Copy link
Member

tkyc commented Jun 17, 2024

We would rather have a fix put in place by Microsoft for this issue. And it would be great to understand when we might expect a fix for this so that we can plan accordingly.

The hotfix will likely be sometime this month. I don't have an exact date yet, but I'll keep you posted.

@tkyc
Copy link
Member

tkyc commented Jun 18, 2024

A tentative hotfix is scheduled for June 20th.

@brentryan
Copy link
Author

Awesome! Thank you!

@lilgreenbird lilgreenbird added the Bug A bug in the driver. A high priority item that one can expect to be addressed quickly. label Jun 19, 2024
@lilgreenbird lilgreenbird added this to the 12.6.3 milestone Jun 19, 2024
@Jeffery-Wasty Jeffery-Wasty linked a pull request Jun 19, 2024 that will close this issue
@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
Bug A bug in the driver. A high priority item that one can expect to be addressed quickly.
Projects
Status: Closed Issues
Development

Successfully merging a pull request may close this issue.

3 participants