Closed bisenpiyush closed 4 years ago
Can the link to Microsoft documentation on "Display size" be provided as part of the section "Changes in this pull request"? Also, testing with large decimal digits and negative numbers should be carried out.
Can the link to Microsoft documentation on "Display size" be provided as part of the section "Changes in this pull request"? Added the link Also, testing with large decimal digits and negative numbers should be carried out. Manual testing has been done already for large decimal value ( for precision value more than 5) and for negative value as well ( for precision value 3, when the scale is 2)
Could you add a few unit tests for this code path (for example, test different decimal values)?
The respective unit test will get added into the unit test written for ODBC Driver in snappy-odbc branch and separate PR will get raised for it.
In Snappy C++ native client, to convert the decimal value into a string, the "mpz_sizeinbase" function of "GMP" library gets used, which returns the number of digits present in decimal value. As per its implementation, it could return the exact result or 1 too big. Since this function returns 1 too big (in other words, if there are 2 digits in decimal value, it returns the 3) which turns into the wrong result string. For example, if the column value is "0.66" then output is "6.6"
Changes proposed in this pull request
Added code to resolve the mpz_sizeinbase (GMP library) function's "1 too big" issue. Added "+2" for display size for data type decimal and numeric. Followed the Microsoft document on "Display size" for this change.(https://docs.microsoft.com/en-us/sql/odbc/reference/appendixes/display-size?view=sql-server-ver15)
Patch testing
Manual testing Unit test
Is precheckin with -Pstore clean?
NA
ReleaseNotes changes
NA
Other PRs
Unit test added in snappy-odbc branch https://github.com/SnappyDataInc/snappy-odbc/pull/13