Arithmetic operations with decimals return incorrect results

[OS-veracardoso]

Driver version

12.4.0.jre11

SQL Server version

Microsoft SQL Server 2019 (RTM-CU15) (KB5008996) - 15.0.4198.2 (X64) 
Jan 12 2022 22:30:08 
Copyright (C) 2019 Microsoft Corporation
Express Edition (64-bit) on Linux (Ubuntu 20.04.3 LTS) <X64>

Client Operating System

Ubuntu on Windows version 2004.2022.1.0

JAVA/JVM version

1.17

Table schema

CREATE TABLE dbo.df_test_all_types
(
    ID           INT PRIMARY KEY,
    COL_TINYINT  TINYINT,
    COL_SMALLINT SMALLINT,
    COL_INT      INT,
    COL_BIGINT   BIGINT,
    COL_FLOAT    FLOAT,
    COL_NUMERIC  NUMERIC,
    COL_DECIMAL  DECIMAL(19, 8),
    COL_BOOLEAN  BIT,
    COL_DATE     DATE,
    COL_TIME     TIME,
    COL_DATETIME DATETIME,
    COL_VARCHAR  VARCHAR(100),
    COL_NVARCHAR NVARCHAR(100),
    COL_TEXT     TEXT,
    COL_BLOB     VARBINARY(5000)
);


INSERT INTO dbo.df_test_all_types (ID, COL_INT, COL_BIGINT, COL_DECIMAL, COL_BOOLEAN,
                                   COL_VARCHAR, COL_NVARCHAR, COL_TEXT)
VALUES (1, 1, 1000, 48.9, 0, 'Some string', 'Some other string', 'Some Text');
INSERT INTO dbo.df_test_all_types (ID, COL_INT, COL_BIGINT, COL_DECIMAL, COL_BOOLEAN,
                                   COL_TEXT)
VALUES (2, 2, 2000, -48.999, 1, ' TrimText ');
INSERT INTO dbo.df_test_all_types (ID, COL_INT, COL_BIGINT, COL_DECIMAL, COL_BOOLEAN,
                                   COL_VARCHAR, COL_NVARCHAR, COL_TEXT)
VALUES (3, 3, 3000, 25, 1, '1234', '1234', '1234');
INSERT INTO dbo.df_test_all_types (ID, COL_INT, COL_BIGINT, COL_DECIMAL, COL_BOOLEAN,
                                   COL_VARCHAR, COL_NVARCHAR, COL_TEXT)
VALUES (4, 4, 4000, 12.34, 1, '2147483650', '2147483650', '2147483650');
INSERT INTO dbo.df_test_all_types (ID, COL_INT, COL_BIGINT, COL_DECIMAL)
VALUES (5, 5, 10, 2.5);
INSERT INTO dbo.df_test_all_types (ID, COL_TINYINT)
VALUES (6, 19);

Problem description

This new version of the driver introduces some changes on how arithmetic operations with decimals work.
For a better understanding consider the follwing examples.

Sum of 3 big decimals with different precision rounds the value to the closest integer:

Query: SELECT ? + ? + ? TABLE_EXAMPLE WHERE "ID" = 4
Dynamic Parameters: new BigDecimal("4"), new BigDecimal("5") and new BigDecimal("3.14")
Expected Result: 5.86
Actual Result: 6

Multiplication followed by a Subtraction of 3 big decimals with different precision returns a decimal without precision:

Query: SELECT ? * ? - ? FROM TABLE_EXAMPLE WHERE "ID" = 4
Dynamic Parameters: new BigDecimal("4"), new BigDecimal("2") and new BigDecimal("1.00")
Expected Result: 7.00
Actual Result: 7

Subtraction of 2 big decimals rounds the result to the closest integer:

Query: SELECT ? - ? FROM TABLE_EXAMPLE WHERE "ID" = 4
Dynamic Parameters: new BigDecimal("4.0") and new BigDecimal("3.14")
Expected Result: 0.86
Actual Result: 1

Subtraction of 2 big decimals round the result to a decimal with the lowest precision:

Example 1

Query: SELECT ? - ? TABLE_EXAMPLE WHERE "ID" = 4
Dynamic Parameters: new BigDecimal("4.0") and new BigDecimal("3.14")
Expected Result: 0.86
Actual Result: 0.9

Example 2

Query: SELECT ? - ? TABLE_EXAMPLE WHERE "ID" = 4]
Dynamic Parameters: new BigDecimal("1.23") and new BigDecimal("1.2")
Expected Result: 0.03
Actual Result: 0.0

Incorrect precision when using CAST

Example 1

Query: SELECT ? - CAST("COL_INT" AS DECIMAL(38, 2) FROM TABLE_EXAMPLE WHERE "ID" = 4
Dynamic Parameters: new BigDecimal("0.00")
Expected Result: 0.00
Actual Result: 0

Example 2

Query: SELECT ? - CAST("COL_BIGINT" AS DECIMAL(38, 2) FROM TABLE_EXAMPLE WHERE "ID" = 4
Dynamic Parameters: new BigDecimal("-3996.00")
Expected Result: -3996.00
Actual Result: -3996

Example 3

Query: SELECT ? - "COL_DECIMAL" FROM TABLE_EXAMPLE WHERE "ID" = 4
Dynamic Parameters: new BigDecimal("-3996.00")
Expected Result: -8.34000000
Actual Result: -8

Error message/stack trace

Not applicable. The driver returns no error. The result though is no the expected one.

[Jeffery-Wasty]

Hi @OS-veracardoso,

For driver versions up until 12.2, and for version 12.4 onwards, BigDecimal precision and scaling values need to be set using setBigDecimal when using a prepared or callable statement. The behavior was "correct" from 12.2 through 12.4, where the driver calculated correct precision and scaling based on the input parameter, however this also introduced major performance problems. As a result, we have decided to roll back how BigDecimal values are handled to the pre-12.2 way, which means users need to use setBigDecimal to ensure proper results following mathematical operations.

This same information can be found in our docs here.

Please let us know if you have any further questions or concerns.

[OS-veracardoso]

Hi @OS-veracardoso,

For driver versions up until 12.2, and for version 12.4 onwards, BigDecimal precision and scaling values need to be set using setBigDecimal when using a prepared or callable statement. The behavior was "correct" from 12.2 through 12.4, where the driver calculated correct precision and scaling based on the input parameter, however this also introduced major performance problems. As a result, we have decided to roll back how BigDecimal values are handled to the pre-12.2 way, which means users need to use setBigDecimal to ensure proper results following mathematical operations.

This same information can be found in our docs here.

Please let us know if you have any further questions or concerns.

Hi @Jeffery-Wasty ,
On all the use cases described, the dynamic parameter is being set using setBigDecimal on the preparedStatement. This is how it looks like:

   preparedStatement -> {
           preparedStatement.setBigDecimal(1, new BigDecimal(left));
           preparedStatement.setBigDecimal(2, new BigDecimal(right));
  }

[Jeffery-Wasty]

What happens if you explicitly pass in precision and scale as below?

   preparedStatement -> {
           preparedStatement.setBigDecimal(1, new BigDecimal(left), left.precision, left.scale);
           preparedStatement.setBigDecimal(2, new BigDecimal(right), right.precision, right.scale);
  }

[OS-veracardoso]

What happens if you explicitly pass in precision and scale as below?

   preparedStatement -> {
           preparedStatement.setBigDecimal(1, new BigDecimal(left), left.precision, left.scale);
           preparedStatement.setBigDecimal(2, new BigDecimal(right), right.precision, right.scale);
  }

Hi @Jeffery-Wasty,
That signature is not available (https://docs.oracle.com/javase%2F7%2Fdocs%2Fapi%2F%2F/java/sql/PreparedStatement.html).
Neverthless, the scale and precision are correctly set on the BigDecimal that is being set, as you can confirm in the follwing screenshot:

[Jeffery-Wasty]

I confused PreparedStatement with the JDBC SQLServerPreparedStatement. Please either use SQLServerPreparedStatement or cast PreparedStatement to SQLServerPreparedStatement.

Even when precision and scale are correctly for the value, unless they are passed in with setBigDecimal the driver has no way of knowing what they are and will assume values that can cause truncation issues.

[OS-veracardoso]

I confused PreparedStatement with the JDBC SQLServerPreparedStatement. Please either use SQLServerPreparedStatement or cast PreparedStatement to SQLServerPreparedStatement.

Even when precision and scale are correctly for the value, unless they are passed in with setBigDecimal the driver has no way of knowing what they are and will assume values that can cause truncation issues.

Hi @Jeffery-Wasty the application we use supports multiple drivers, so the prepared statement logic is kind of abstract and only uses the available signatures, so we cannot do the cast that you suggest.
As mentioned on my previous comment, we do use setBigDecimal to set the value on the prepared statement, so the scale and precision are defined on the value set, as displayed in the image on my previous comment.

[Jeffery-Wasty]

As mentioned on my previous comment, we do use setBigDecimal to set the value on the prepared statement, so the scale and precision are defined on the value set, as displayed in the image on my previous comment.

Even when the precision and scale are set correctly in the BigDecimal being passed in, we still require it to be explicitly passed in. Attempting to parse the BigDecimal to get precision/scale leads to major performance impact with the JDBC driver.

If its not possible to use SQLServerPreparedStatement, another option is to use a version of the driver between 12.2 and 12.4. These versions of the driver do calculate precision and scale from BigDecimal, but you may experience additional performance impact that caused us to rollback these changes in 12.4.

We're investigating whether there is another way to proceed with this situation.

[tkyc]

@OS-veracardoso The mssql-jdbc driver's behaviour regarding precision and scale closely mirrors JTDS. And, from when I last checked, JTDS behaves similarily if precision and scale isn't explictily supplied. If this is working for you with other JDBC drivers, likely the mssql-jdbc might be wrong or missing some logic. What are the other drivers you are using in your application?

[tkyc]

I've setup a MySQL DB + mysql-connector-j driver to compare, and it looks like MySQL is behaving correctly. The math operations come out correct despite not explicitly specifying the precision and scale for the prepared statement.

We'll need to do more research and compare the mssql-jdbc driver to MySQL's mysql-connector-j driver.

[OS-veracardoso]

I've setup a MySQL DB + mysql-connector-j driver to compare, and it looks like MySQL is behaving correctly. The math operations come out correct despite not explicitly specifying the precision and scale for the prepared statement.

We'll need to do more research and compare the mssql-jdbc driver to MySQL's mysql-connector-j driver.

Yes, if you try with Oracle, it also returns the expected result.

[OS-veracardoso]

@OS-veracardoso The mssql-jdbc driver's behaviour regarding precision and scale closely mirrors JTDS. And, from when I last checked, JTDS behaves similarily if precision and scale isn't explictily supplied. If this is working for you with other JDBC drivers, likely the mssql-jdbc might be wrong or missing some logic. What are the other drivers you are using in your application?

It works as expected using mysql and oracle JDBC drivers, for instance. This was also working with the previous version of mssql driver. We notice this regression while trying to upgrade to the latest version and our tests started to fail.

[lilgreenbird]

hi @OS-veracardoso

apologies for the delay in response we've been busy with getting ready for a hotfix release.

You are correct this had worked in a previous version of the driver however we backed out of the fix in #1928 when we discovered there was a performance issue with the fix. The issue is that there is no way for the driver to know what the underlying precision/scale is without contacting the server and that would impact performance. We will investigate further to see how this is accomplished by other drivers.

[OS-veracardoso]

hi @OS-veracardoso

apologies for the delay in response we've been busy with getting ready for a hotfix release.

You are correct this had worked in a previous version of the driver however we backed out of the fix in #1928 when we discovered there was a performance issue with the fix. The issue is that there is no way for the driver to know what the underlying precision/scale is without contacting the server and that would impact performance. We will investigate further to see how this is accomplished by other drivers.

@lilgreenbird Thank you.

[Jeffery-Wasty]

Hi @OS-veracardoso,

In addition to the testing we did previously with our own driver, we looked into jTDS and the MySQL Java connector.

For jTDS, it behaves the same as the JDBC driver, producing similar arithmetic errors. For the MySQL connector, they are using a similar approach to what we did in 12.2, being able to parse the BigDecimal to fetch precision and scale from there. We're not sure what kind of performance impact this introduces to the driver, but the same approach in the JDBC driver caused a noticeable performance impact. We tried to find a way that precision could be updated for the parameter type definition without recreating the whole execution plan (the reason for the performance impact) but were unable to do so.

For the time being, we ask that you consider using the 12.2 version of the driver, as that version has the BigDecimal fixes you are interested in. A potential enhancement to the driver could include a connection property to restore previous BigDecimal behavior, at the cost of performance. We'll keep this issue open while that option is still in consideration/development.

[OS-veracardoso]

Hello,
I just tested the new version of the driver, and some operations are still not behaving like they used to in version 12.3.1.jre11.

Example1:

Query: SELECT TOP (1) ?  + ?  - ? TABLE_EXAMPLE
Dynamic Parameters: new BigDecimal("4"), new BigDecimal("5") and new BigDecimal("3.14")
Expected Result: 5.86
Actual Result: 6

Example 2:

Query: SELECT TOP (1) ?  * ?  - ? TABLE_EXAMPLE 
Dynamic Parameters: new BigDecimal("4"), new BigDecimal("2") and new BigDecimal("1.00")
Expected Result: 7.00
Actual Result: 7

Example 3:

Query: SELECT TOP (1) ? - ? TABLE_EXAMPLE
Dynamic Parameters: new BigDecimal("4.0") and new BigDecimal("3.14")
Expected Result: 0.86
Actual Result: 0.9

Example 4:

Query: SELECT ? - "COL_DECIMAL" FROM TABLE_EXAMPLE WHERE "ID" = 4
Dynamic Parameters: new BigDecimal("4")
Expected Result: -8.34000000
Actual Result: -8

We also realized that the precision with decimal division changed. For isntance, the query SELECT ? / ? FROM TABLE_EXAMPLE with

ps.setInt(1, 3);
ps.setBigDecimal(2, new BigDecimal("2"));
``
used to return `1.500000` and now it returns `1.5000000000000000000000000000`.

[tkyc]

@OS-veracardoso For the latest preview release 12.5.0, you'll need need to use the new boolean connection property calcBigDecimalScale. Setting this to true should apply the old 12.3.1 behaviour.

[Jeffery-Wasty]

Hi @OS-veracardoso,

I will take another look, but the code is identical to 12.3.1 (with the added condition being that the connection property must be set). If you're able to confirm that the preview release, 12.5.0, is not working the same as the former version, 12.3.1, with the connection property enabled, that would be something we would need to resolve.

[OS-veracardoso]

Hi @Jeffery-Wasty, @tkyc,

Adding the mentioned property to the connection string works as expected. Thank you for your help.

[OS-veracardoso]

Hello, this is now happening again in version 12.6.0.jre11. Could you please take a look?

[Jeffery-Wasty]

Hi @OS-veracardoso,

We changed the name to calcBigDecimalPrecision (it was improperly named before). Are you setting that to true in your connection properties?

[OS-veracardoso]

Hi @OS-veracardoso,

We changed the name to calcBigDecimalPrecision (it was improperly named before). Are you setting that to true in your connection properties?

Hi @Jeffery-Wasty, thanks. It is working now.