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

Precision of numeric columns is not set correctly for TVP with multiple rows #211

Closed
jneubecker opened this issue Mar 21, 2017 · 9 comments
Assignees

Comments

@jneubecker
Copy link

jneubecker commented Mar 21, 2017

There is a bug with inserting numeric or decimal values with a table valued parameter in the way precision is being set. For example when inserting 2 rows into a table with a numeric(6,3) column, if the value in the first row for the numeric column is 12.12 and the value for the same column in the second row is 1.123 the precision and scale in SQLServerDataTable for that column are set to 4 and 3. And when the metadata is populated in TVP.populateMetadataFromTable the precision and scale are then set to 4 and 3 for that column. But 12.12 is not a valid numeric(4,3). In SQLServerDataTable the precision should be set to the maximum of the precision to the left plus the maximum of the precision right of the decimal point between all values for a column. One way to do this would be to increase the precision when the scale is increased by the difference of the old and new scale.

Given the table:

CREATE TABLE [dbo].[TEST_TABLE](
	[id] [numeric](18, 0) NOT NULL,
	[number] [numeric](6, 3) NULL
)

And the table type:

CREATE TYPE [dbo].[testTableType] AS TABLE(
	[id] [numeric](18, 0) NULL,
	[number] [numeric](6, 3) NULL,
)

The following code will reproduce the error:

try (SQLServerPreparedStatement preparedStatement = (SQLServerPreparedStatement) conn.prepareStatement(“insert into TEST_TABLE (id, number) select TMP1.id, TMP1.number from ? as TMP1”)) {
      SQLServerDataTable dataTable = new SQLServerDataTable();

      dataTable.addColumnMetadata(“id”, java.sql.Types.NUMERIC);
      dataTable.addColumnMetadata(“number”, java.sql.Types.NUMERIC);

      dataTable.addRow(1, 12.12);
      dataTable.addRow(2, 1.123);

      preparedStatement.setStructured(1, “testTableType”, dataTable);

      preparedStatement.execute();
}
com.microsoft.sqlserver.jdbc.SQLServerException: The incoming tabular data stream (TDS) remote procedure call (RPC) protocol stream is incorrect. Table-valued parameter 1 (""), row 1, column 2: The supplied value is not a valid instance of data type numeric. Check the source data for invalid values. An example of an invalid value is data of numeric type with scale greater than precision.
@xiangyushawn xiangyushawn self-assigned this Mar 22, 2017
@xiangyushawn xiangyushawn added the Under Review Used for pull requests under review label Mar 22, 2017
@xiangyushawn
Copy link
Contributor

@jneubecker Thank you for reporting the issue. This is really a valuable finding for us. We also really appreciate the very detailed repro code you provided to us. We are looking at it and will get back to you soon.

@xiangyushawn
Copy link
Contributor

@jneubecker We have created a PR to fix it. Hope this works for you. Thank you again for finding and reporting the issue.

@xiangyushawn xiangyushawn added Waiting for Response Waiting for a reply from the original poster, or affiliated party and removed Under Review Used for pull requests under review labels Mar 22, 2017
@gordthompson
Copy link
Contributor

It seems to me that we should be careful about feeding decimal literals into a SQLServerDataTable, lest those values be interpreted as Double. If they are subsequently converted to BigDecimal then the .precision and .scale may not be what we expect. For example

BigDecimal bd = new BigDecimal("3.14159");
System.out.printf("That value looks like DECIMAL(%d,%d)", bd.precision(), bd.scale());

produces

That value looks like DECIMAL(6,5)

as expected, but

BigDecimal bd = new BigDecimal(3.14159);
System.out.printf("That value looks like DECIMAL(%d,%d)", bd.precision(), bd.scale());

produces

That value looks like DECIMAL(51,50)

@xiangyushawn
Copy link
Contributor

@gordthompson Thank you for the note. You are totally right, we need to be careful to use String value when converting a decimal value into BigDecimal, exactly like what the driver does

@jschulist
Copy link

Looks like this is related to issue #86 that I opened sometime back

@xiangyushawn
Copy link
Contributor

hello @jschulist , thank you for commenting on this issue. Those 2 issues are very similar, but still different :) And we have already merged the PR made for issue #86

@jschulist
Copy link

Yes, agreed, the PR for #86 missed this case. It was not something I directly encountered in my testing either.

@jneubecker
Copy link
Author

Thanks for the quick turn-around on the issue. When is the fix likely to be released?

@xiangyushawn
Copy link
Contributor

@jneubecker Thank you for getting back to us. If you are in a hurry, the next preview release will be on Maven within one week. It may take longer for production quality release.

@v-nisidh v-nisidh removed the Waiting for Response Waiting for a reply from the original poster, or affiliated party label Apr 26, 2017
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

No branches or pull requests

5 participants