Skip to content

Fixes issue with PreparedStatement.setBigDecimal() in the driver when no scale is passed#684

Merged
cheenamalhotra merged 9 commits intomicrosoft:devfrom
cheenamalhotra:bigdecimal-issue
May 2, 2018
Merged

Fixes issue with PreparedStatement.setBigDecimal() in the driver when no scale is passed#684
cheenamalhotra merged 9 commits intomicrosoft:devfrom
cheenamalhotra:bigdecimal-issue

Conversation

@cheenamalhotra
Copy link
Copy Markdown
Member

@cheenamalhotra cheenamalhotra commented Apr 23, 2018

Fix for issue #683.

Issue occurs when BigDecimal value is created from double [e.g. new BigDecimal(5.55)] or as it is exceeds in Precision than maxAllowedPrecision (38) in SQL Server, and no scale is passed from Client. The driver in this case fails to round off digits and passes BigDecimal value as String, which when received by SQL Server fails with Exception:
com.microsoft.sqlserver.jdbc.SQLServerException: Error converting data type nvarchar to decimal.

This fix makes sure BigDecimal values passed to SQL Server are within precision limits and scaled accordingly.

@cheenamalhotra cheenamalhotra mentioned this pull request Apr 23, 2018
@codecov-io
Copy link
Copy Markdown

codecov-io commented Apr 23, 2018

Codecov Report

Merging #684 into dev will decrease coverage by <.01%.
The diff coverage is 45.45%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #684      +/-   ##
============================================
- Coverage     48.13%   48.13%   -0.01%     
+ Complexity     2576     2573       -3     
============================================
  Files           113      113              
  Lines         26561    26569       +8     
  Branches       4435     4438       +3     
============================================
+ Hits          12786    12789       +3     
+ Misses        11647    11642       -5     
- Partials       2128     2138      +10
Flag Coverage Δ Complexity Δ
#JDBC42 48% <45.45%> (-0.05%) 2567 <0> (-4)
#JDBC43 48.1% <45.45%> (+0.05%) 2572 <0> (+2) ⬆️
Impacted Files Coverage Δ Complexity Δ
...rc/main/java/com/microsoft/sqlserver/jdbc/dtv.java 63.41% <45.45%> (+0.23%) 0 <0> (ø) ⬇️
...om/microsoft/sqlserver/jdbc/ReaderInputStream.java 42.69% <0%> (-4.5%) 14% <0%> (-3%)
...n/java/com/microsoft/sqlserver/jdbc/Parameter.java 61.92% <0%> (-0.84%) 63% <0%> (ø)
...rc/main/java/com/microsoft/sqlserver/jdbc/DDC.java 44.96% <0%> (-0.45%) 107% <0%> (ø)
...oft/sqlserver/jdbc/SQLServerPreparedStatement.java 50.6% <0%> (-0.25%) 157% <0%> (ø)
...m/microsoft/sqlserver/jdbc/SQLServerStatement.java 59.51% <0%> (-0.11%) 131% <0%> (-1%)
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 54.79% <0%> (+0.19%) 0% <0%> (ø) ⬇️
...om/microsoft/sqlserver/jdbc/SimpleInputStream.java 52.59% <0%> (+1.48%) 12% <0%> (+1%) ⬆️

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 4a9c840...b9062ba. Read the comment docs.

Copy link
Copy Markdown
Contributor

@peterbae peterbae left a comment

Choose a reason for hiding this comment

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

I would put the if statement at line 2215 in a bracket (we should apply brackets to if statements even if it only has one line).

Otherwise looks good to me. We won't have to worry about the line 2216 turning up negative since SQLServerConnectionmaxDecimalPrecision will always be larger than bigDecimalValue.precision().

@cheenamalhotra cheenamalhotra added this to the 6.5.2 milestone Apr 28, 2018
Integer inScale = dtv.getScale();
if (null != inScale && inScale != bigDecimalValue.scale())
bigDecimalValue = bigDecimalValue.setScale(inScale, RoundingMode.DOWN);
Integer dtvScale, biScale = bigDecimalValue.scale();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please apply the formatter, otherwise looks good!

@cheenamalhotra cheenamalhotra merged commit 55bc4a0 into microsoft:dev May 2, 2018
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.

5 participants