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

fix for queries without parameters using preparedStatement #372

Merged

Conversation

AfsanehR-zz
Copy link
Contributor

@AfsanehR-zz AfsanehR-zz commented Jul 6, 2017

Fixes #370

@codecov-io
Copy link

codecov-io commented Jul 7, 2017

Codecov Report

Merging #372 into RTW_6.2.0 will decrease coverage by 0.11%.
The diff coverage is 100%.

Impacted file tree graph

@@               Coverage Diff               @@
##             RTW_6.2.0     #372      +/-   ##
===============================================
- Coverage        40.27%   40.16%   -0.12%     
+ Complexity        1896     1886      -10     
===============================================
  Files              107      107              
  Lines            24486    24487       +1     
  Branches          4039     4039              
===============================================
- Hits              9862     9834      -28     
- Misses           12786    12816      +30     
+ Partials          1838     1837       -1
Flag Coverage Δ Complexity Δ
#JDBC41 40.01% <100%> (-0.11%) 1879 <0> (-9)
#JDBC42 39.99% <100%> (-0.2%) 1877 <0> (-13)
Impacted Files Coverage Δ Complexity Δ
...oft/sqlserver/jdbc/SQLServerPreparedStatement.java 42.43% <100%> (+0.04%) 136 <0> (ø) ⬇️
src/main/java/microsoft/sql/DateTimeOffset.java 37.14% <0%> (-2.86%) 8% <0%> (-2%)
...a/com/microsoft/sqlserver/jdbc/PLPInputStream.java 39.75% <0%> (-1.21%) 28% <0%> (-1%)
...c/main/java/com/microsoft/sqlserver/jdbc/Util.java 47.51% <0%> (-1.14%) 63% <0%> (-2%)
...om/microsoft/sqlserver/jdbc/ReaderInputStream.java 44.94% <0%> (-1.13%) 16% <0%> (-1%)
...m/microsoft/sqlserver/jdbc/SQLServerResultSet.java 27.23% <0%> (-0.42%) 187% <0%> (-4%)
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 44.99% <0%> (-0.38%) 0% <0%> (ø)
...ncurrentlinkedhashmap/ConcurrentLinkedHashMap.java 42.2% <0%> (-0.22%) 46% <0%> (ø)
...rc/main/java/com/microsoft/sqlserver/jdbc/dtv.java 39.59% <0%> (-0.07%) 0% <0%> (ø)
...om/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java 49.5% <0%> (+0.14%) 210% <0%> (-1%) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6acd67c...97a597a. Read the comment docs.

@v-nisidh v-nisidh requested a review from TobiasSQL July 7, 2017 01:07
@v-nisidh v-nisidh added this to the 6.2.1 milestone Jul 7, 2017
@@ -966,6 +966,7 @@ private boolean doPrepExec(TDSWriter tdsWriter,
if (needsPrepare
&& !connection.getEnablePrepareOnFirstPreparedStatementCall()
&& !isExecutedAtLeastOnce
&& preparedTypeDefinitions.length() > 0
Copy link
Contributor

@TobiasSQL TobiasSQL Jul 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not make this check here. sp_executesql works fine without arguments. The problem is in the buildExecSQLParams method. Basically this works fine for sp_prepexec/buildPrepExecParams but not for sp_executesql:
tdsWriter.writeRPCStringUnicode((preparedTypeDefinitions.length() > 0) ? preparedTypeDefinitions : null);

So we should just change that to:
if (preparedTypeDefinitions.length() > 0) tdsWriter.writeRPCStringUnicode(preparedTypeDefinitions);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks @TobiasSQL . That was my initial thought too. Will commit the changes.

@AfsanehR-zz AfsanehR-zz merged commit 2fa1553 into microsoft:RTW_6.2.0 Jul 12, 2017
@AfsanehR-zz AfsanehR-zz deleted the metaDataCachingRegression branch February 2, 2018 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants