gty404 commented on code in PR #1084:
URL: https://github.com/apache/iceberg-cpp/pull/1084#discussion_r4475637034
##########
src/iceberg/util/decimal.cc:
##########
@@ -552,12 +552,26 @@ Result<Decimal> Decimal::Rescale(int32_t orig_scale,
int32_t new_scale) const {
return *this;
}
- const int32_t delta_scale = new_scale - orig_scale;
+ // Compute the delta in 64 bits so unvalidated scales cannot cause signed
Review Comment:
Yes — `BasicDecimal128::GetScaleMultiplier` in Arrow's `basic_decimal.cc`
also only has `DCHECK_GE/DCHECK_LE`, a no-op in release builds, before indexing
`kDecimal128PowersOfTen[scale]`, and `DecimalRescale`'s `delta_scale =
new_scale - original_scale` has the same unguarded overflow. Arrow has no
scale-aware `Compare` at this layer though, so that part of this fix is
iceberg-cpp-specific.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
