Fix invalid read in tsql_round_var function #3720
Conversation
When the rscale is negative and numeric value is 0 we can skip checking for numeric overflow as the base-NBASE digits will be empty for 0
Pull Request Test Coverage Report for Build 14620310275Details
💛 - Coveralls |
| go | ||
| ~~START~~ | ||
| numeric | ||
| 0 |
There was a problem hiding this comment.
Is this also what SQL Server outputs ?
There was a problem hiding this comment.
In sqlserver it includes decimal
1> select round(0.0, -1)
2> go
---
0.0
But currently in babelfish it doesn't show decimal with/without this change as well
There was a problem hiding this comment.
Can we figure out what the RCA is for this issue. I am just trying to confirm if this needs to be handled in tsql_round_var
There was a problem hiding this comment.
They are forcing it to 0 here
https://github.com/babelfish-for-postgresql/babelfish_extensions/blob/BABEL_5_X_DEV/contrib/babelfishpg_common/src/numeric.c#L672
So we have to handle it here I think
There was a problem hiding this comment.
We have jira for correct scale from round function BABEL-5666.
Maybe this will be handled in resolve_numeric_typmod_from_exp.
There was a problem hiding this comment.
@sharathbp could we add this query in the comments of that jira so we don't miss while fixing that change.
| go | ||
| ~~START~~ | ||
| numeric | ||
| 0 |
There was a problem hiding this comment.
Can we figure out what the RCA is for this issue. I am just trying to confirm if this needs to be handled in tsql_round_var
shardgupta
merged commit eb1213e
into
babelfish-for-postgresql:BABEL_5_X_DEV
Description
To detect for overflow and underflow we need to determine total integral digits which in-turn uses leading digits in the digits array. But digits array will be empty in case of 0 numeric values which leads to invalid read.
When the rscale is negative and numeric value is 0 we can skip checking for numeric overflow as the base-NBASE digits will be empty for 0.
Issues Resolved
BABEL-5546
Test Scenarios Covered
Use case based -
Yes
Boundary conditions -
Yes
Arbitrary inputs -
Yes
Negative test cases -
Yes
Minor version upgrade tests -
NA
Major version upgrade tests -
NA
Performance tests -
NA
Tooling impact -
NA
Client tests -
NA
Check List
By submitting this pull request, I confirm that my contribution is under the terms of the Apache 2.0 and PostgreSQL licenses, and grant any person obtaining a copy of the contribution permission to relicense all or a portion of my contribution to the PostgreSQL License solely to contribute all or a portion of my contribution to the PostgreSQL open source project.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.