etseidl commented on code in PR #37940:
URL: https://github.com/apache/arrow/pull/37940#discussion_r1341518062
##########
cpp/src/parquet/encoding.cc:
##########
@@ -2212,12 +2229,12 @@ void DeltaBitPackEncoder<DType>::Put(const T* src, int
num_values) {
total_value_count_ += num_values;
while (idx < num_values) {
- UT value = static_cast<UT>(src[idx]);
- // Calculate deltas. The possible overflow is handled by use of unsigned
integers
+ T value = src[idx];
+ // Calculate deltas. The possible overflow is handled by use of wide
integers
// making subtraction operations well-defined and correct even in case of
overflow.
// Encoded integers will wrap back around on decoding.
// See http://en.wikipedia.org/wiki/Modular_arithmetic#Integers_modulo_n
- deltas_[values_current_block_] = value - current_value_;
+ deltas_[values_current_block_] = SafeSignedSubtractSigned(value,
current_value_);
Review Comment:
Yes, you're right, we don't need a new safe subtract. The existing
`SafeSignedSubtract` does the subtraction with unsigned to get the correct
wrapping behavior, and then casts back to signed. No need for promotion to the
larger type. I've changed to `SafeSignedSubtract` everywhere.
--
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]
