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 issue 705 by including the possible negative sign #732

Merged
merged 2 commits into from
Apr 4, 2018
Merged

Fix issue 705 by including the possible negative sign #732

merged 2 commits into from
Apr 4, 2018

Conversation

yitam
Copy link
Contributor

@yitam yitam commented Apr 2, 2018

This change is Reviewable

@coveralls
Copy link

coveralls commented Apr 2, 2018

Coverage Status

Coverage decreased (-0.03%) to 73.301% when pulling 88f0bcf on yitam:issue705 into 1958cfd on Microsoft:dev.

@codecov-io
Copy link

codecov-io commented Apr 2, 2018

Codecov Report

Merging #732 into dev will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #732      +/-   ##
==========================================
- Coverage   79.02%   78.99%   -0.03%     
==========================================
  Files          25       25              
  Lines        7188     7190       +2     
==========================================
  Hits         5680     5680              
- Misses       1508     1510       +2
Impacted Files Coverage Δ
...p-7.1.15-src/ext/pdo_sqlsrv/shared/core_stream.cpp
...rojects/php/x86/php-7.1.15-src/ext/sqlsrv/stmt.cpp
...php/x86/php-7.1.15-src/ext/pdo_sqlsrv/pdo_util.cpp
...rojects/php/x86/php-7.1.15-src/ext/sqlsrv/conn.cpp
...php-7.1.15-src/ext/pdo_sqlsrv/shared/core_conn.cpp
...php-7.1.15-src/ext/pdo_sqlsrv/shared/core_util.cpp
...php-7.1.15-src/ext/pdo_sqlsrv/shared/core_sqlsrv.h
...p/x86/php-7.1.15-src/ext/pdo_sqlsrv/pdo_parser.cpp
...x86/php-7.1.15-src/ext/sqlsrv/shared/core_util.cpp
.../php-7.1.15-src/ext/sqlsrv/shared/core_results.cpp
... and 40 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 1958cfd...88f0bcf. Read the comment docs.

@yitam yitam requested a review from david-puglielli April 2, 2018 23:37
if ( stmt->conn->ce_option.enabled && decimal_digits > 0 )
{
// with AE on, when column_size is retrieved from SQLDescribeParam, column_size does not include the negative sign or decimal place for numeric values
if (stmt->conn->ce_option.enabled && (sql_type == SQL_DECIMAL || sql_type == SQL_NUMERIC || sql_type == SQL_INTEGER || sql_type == SQL_SMALLINT)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it not an issue with floats, reals, or bigints?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point about bigints but not an issue with floats or reals when testing

@yitam yitam merged commit dd58240 into microsoft:dev Apr 4, 2018
@yitam yitam deleted the issue705 branch April 4, 2018 21:46
@yitam
Copy link
Contributor Author

yitam commented Apr 4, 2018

Fix for #705

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.

4 participants