wgtmac commented on code in PR #1500:
URL: https://github.com/apache/orc/pull/1500#discussion_r1222450883
##########
c++/src/Int128.cc:
##########
@@ -488,4 +488,84 @@ namespace orc {
return value;
}
+ std::pair<bool, Int128> convertDecimal(Int128 value, int32_t fromScale,
int32_t toPrecision,
+ int32_t toScale, bool round) {
+ std::pair<bool, Int128> result;
+ bool negative = value < 0;
+ result.second = value.abs();
+ result.first = false;
+
+ Int128 upperBound = scaleUpInt128ByPowerOfTen(1, toPrecision,
result.first);
+ int8_t roundOffset = 0;
+ int32_t deltaScale = fromScale - toScale;
+
+ if (deltaScale > 0) {
+ Int128 scale = scaleUpInt128ByPowerOfTen(1, deltaScale, result.first),
remainder;
+ result.second = result.second.divide(scale, remainder);
+ remainder *= 2;
+ if (round && remainder >= scale) {
+ upperBound -= 1;
+ roundOffset = 1;
+ }
+ } else if (deltaScale < 0) {
+ if (result.second > upperBound) {
+ result.first = true;
+ return result;
+ }
+ result.second = scaleUpInt128ByPowerOfTen(result.second, -deltaScale,
result.first);
+ }
+
+ if (result.second > upperBound) {
+ result.first = true;
+ return result;
+ }
+
+ result.second += roundOffset;
+ if (negative) {
+ result.second *= -1;
+ }
+ return result;
+ }
+
+ template <typename T>
Review Comment:
Should we use std::enable_if to restrict T to be floating type only?
##########
c++/src/Int128.cc:
##########
@@ -488,4 +488,84 @@ namespace orc {
return value;
}
+ std::pair<bool, Int128> convertDecimal(Int128 value, int32_t fromScale,
int32_t toPrecision,
+ int32_t toScale, bool round) {
+ std::pair<bool, Int128> result;
+ bool negative = value < 0;
+ result.second = value.abs();
+ result.first = false;
+
+ Int128 upperBound = scaleUpInt128ByPowerOfTen(1, toPrecision,
result.first);
+ int8_t roundOffset = 0;
+ int32_t deltaScale = fromScale - toScale;
+
+ if (deltaScale > 0) {
+ Int128 scale = scaleUpInt128ByPowerOfTen(1, deltaScale, result.first),
remainder;
+ result.second = result.second.divide(scale, remainder);
+ remainder *= 2;
+ if (round && remainder >= scale) {
+ upperBound -= 1;
+ roundOffset = 1;
+ }
+ } else if (deltaScale < 0) {
+ if (result.second > upperBound) {
+ result.first = true;
+ return result;
+ }
+ result.second = scaleUpInt128ByPowerOfTen(result.second, -deltaScale,
result.first);
+ }
+
+ if (result.second > upperBound) {
+ result.first = true;
+ return result;
+ }
+
+ result.second += roundOffset;
+ if (negative) {
+ result.second *= -1;
+ }
+ return result;
+ }
+
+ template <typename T>
+ std::pair<bool, Int128> convertDecimal(T value, int32_t precision, int32_t
scale) {
Review Comment:
Should we check if value is nan?
##########
c++/src/Int128.cc:
##########
@@ -488,4 +488,84 @@ namespace orc {
return value;
}
+ std::pair<bool, Int128> convertDecimal(Int128 value, int32_t fromScale,
int32_t toPrecision,
+ int32_t toScale, bool round) {
+ std::pair<bool, Int128> result;
+ bool negative = value < 0;
+ result.second = value.abs();
+ result.first = false;
+
+ Int128 upperBound = scaleUpInt128ByPowerOfTen(1, toPrecision,
result.first);
+ int8_t roundOffset = 0;
+ int32_t deltaScale = fromScale - toScale;
+
+ if (deltaScale > 0) {
+ Int128 scale = scaleUpInt128ByPowerOfTen(1, deltaScale, result.first),
remainder;
+ result.second = result.second.divide(scale, remainder);
+ remainder *= 2;
+ if (round && remainder >= scale) {
+ upperBound -= 1;
+ roundOffset = 1;
+ }
+ } else if (deltaScale < 0) {
+ if (result.second > upperBound) {
+ result.first = true;
+ return result;
+ }
+ result.second = scaleUpInt128ByPowerOfTen(result.second, -deltaScale,
result.first);
+ }
+
+ if (result.second > upperBound) {
+ result.first = true;
+ return result;
+ }
+
+ result.second += roundOffset;
+ if (negative) {
+ result.second *= -1;
+ }
+ return result;
+ }
+
+ template <typename T>
+ std::pair<bool, Int128> convertDecimal(T value, int32_t precision, int32_t
scale) {
+ std::pair<bool, Int128> result = {false, 0};
+ if (value <= -std::ldexp(static_cast<T>(1), 127) ||
Review Comment:
Make std::ldexp(static_cast<T>(1), 127) const static as it is repeated used.
##########
c++/test/TestInt128.cc:
##########
@@ -643,4 +643,242 @@ namespace orc {
EXPECT_TRUE(Int128(-12340000).toDecimalString(8, true) == "-0.1234");
}
+ TEST(Int128, testConvertDecimal) {
+ // Test convert decimal to different precision/scale
+ Int128 num = Int128(1234567890);
+
+ int fromScale = 5;
+ int toPrecision = 9;
+ int toScale = 5;
+ auto pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true) << pair.second.toString(); // overflow
+
+ fromScale = 5;
+ toPrecision = 9;
+ toScale = 4;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(123456789));
+
+ fromScale = 5;
+ toPrecision = 9;
+ toScale = 3;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345679)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 10;
+ toScale = 0;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12346));
+
+ fromScale = 5;
+ toPrecision = 10;
+ toScale = 2;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(1234568)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 10;
+ toScale = 5;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(1234567890)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 10;
+ toScale = 6;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true);
+
+ fromScale = 5;
+ toPrecision = 11;
+ toScale = 0;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12346));
+
+ fromScale = 5;
+ toPrecision = 11;
+ toScale = 3;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345679)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 11;
+ toScale = 6;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345678900)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 11;
+ toScale = 7;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true); // overflow
+
+ fromScale = 5;
+ toPrecision = 12;
+ toScale = 5;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(1234567890)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 12;
+ toScale = 6;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345678900)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 12;
+ toScale = 8;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true); // overflow
+ }
+
+ TEST(Int128, testConvertDecimaFromFloat) {
+ double fromDouble = 12345.13579;
+ int toPrecision = 4;
+ int toScale = 2;
+ auto pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true); // overflow
+
+ toPrecision = 5;
+ toScale = 0;
+ pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345)) << pair.second.toString();
+
+ toPrecision = 5;
+ toScale = 1;
+ pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true); // overflow
+
+ toPrecision = 6;
+ toScale = 0;
+ pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345)) << pair.second.toString();
+
+ toPrecision = 6;
+ toScale = 1;
+ pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(123451)) << pair.second.toString();
+
+ toPrecision = 6;
+ toScale = 2;
+ pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true); // overflow
+
+ toPrecision = 8;
+ toScale = 3;
+ pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345136)) << pair.second.toString();
+
+ fromDouble = -12345.13579;
Review Comment:
Probably +0.0, -0.0, and NaN should be added
##########
c++/src/Int128.cc:
##########
@@ -488,4 +488,84 @@ namespace orc {
return value;
}
+ std::pair<bool, Int128> convertDecimal(Int128 value, int32_t fromScale,
int32_t toPrecision,
+ int32_t toScale, bool round) {
+ std::pair<bool, Int128> result;
+ bool negative = value < 0;
+ result.second = value.abs();
+ result.first = false;
+
+ Int128 upperBound = scaleUpInt128ByPowerOfTen(1, toPrecision,
result.first);
Review Comment:
Or assert fromScale, toPrecision, toScale are in the valid ranges.
##########
c++/test/TestInt128.cc:
##########
@@ -643,4 +643,242 @@ namespace orc {
EXPECT_TRUE(Int128(-12340000).toDecimalString(8, true) == "-0.1234");
}
+ TEST(Int128, testConvertDecimal) {
+ // Test convert decimal to different precision/scale
+ Int128 num = Int128(1234567890);
+
+ int fromScale = 5;
+ int toPrecision = 9;
+ int toScale = 5;
+ auto pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true) << pair.second.toString(); // overflow
+
+ fromScale = 5;
+ toPrecision = 9;
+ toScale = 4;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(123456789));
+
+ fromScale = 5;
+ toPrecision = 9;
+ toScale = 3;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345679)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 10;
+ toScale = 0;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12346));
+
+ fromScale = 5;
+ toPrecision = 10;
+ toScale = 2;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(1234568)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 10;
+ toScale = 5;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(1234567890)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 10;
+ toScale = 6;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true);
+
+ fromScale = 5;
+ toPrecision = 11;
+ toScale = 0;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12346));
+
+ fromScale = 5;
+ toPrecision = 11;
+ toScale = 3;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345679)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 11;
+ toScale = 6;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345678900)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 11;
+ toScale = 7;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true); // overflow
+
+ fromScale = 5;
+ toPrecision = 12;
+ toScale = 5;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(1234567890)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 12;
+ toScale = 6;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345678900)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 12;
+ toScale = 8;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true); // overflow
+ }
+
+ TEST(Int128, testConvertDecimaFromFloat) {
+ double fromDouble = 12345.13579;
+ int toPrecision = 4;
+ int toScale = 2;
+ auto pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true); // overflow
+
+ toPrecision = 5;
+ toScale = 0;
+ pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345)) << pair.second.toString();
+
+ toPrecision = 5;
+ toScale = 1;
+ pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true); // overflow
+
+ toPrecision = 6;
+ toScale = 0;
+ pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345)) << pair.second.toString();
+
+ toPrecision = 6;
+ toScale = 1;
+ pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(123451)) << pair.second.toString();
+
+ toPrecision = 6;
+ toScale = 2;
+ pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true); // overflow
+
+ toPrecision = 8;
+ toScale = 3;
+ pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345136)) << pair.second.toString();
+
+ fromDouble = -12345.13579;
Review Comment:
Could you add more numbers to test? Would be good to test float type as well.
##########
c++/src/Int128.cc:
##########
@@ -488,4 +488,84 @@ namespace orc {
return value;
}
+ std::pair<bool, Int128> convertDecimal(Int128 value, int32_t fromScale,
int32_t toPrecision,
+ int32_t toScale, bool round) {
+ std::pair<bool, Int128> result;
+ bool negative = value < 0;
+ result.second = value.abs();
+ result.first = false;
+
+ Int128 upperBound = scaleUpInt128ByPowerOfTen(1, toPrecision,
result.first);
Review Comment:
nit: check result.first in case of invalid toPrecision
##########
c++/test/TestInt128.cc:
##########
@@ -643,4 +643,242 @@ namespace orc {
EXPECT_TRUE(Int128(-12340000).toDecimalString(8, true) == "-0.1234");
}
+ TEST(Int128, testConvertDecimal) {
+ // Test convert decimal to different precision/scale
+ Int128 num = Int128(1234567890);
+
+ int fromScale = 5;
+ int toPrecision = 9;
+ int toScale = 5;
+ auto pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true) << pair.second.toString(); // overflow
+
+ fromScale = 5;
+ toPrecision = 9;
+ toScale = 4;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(123456789));
+
+ fromScale = 5;
+ toPrecision = 9;
+ toScale = 3;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345679)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 10;
+ toScale = 0;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12346));
+
+ fromScale = 5;
+ toPrecision = 10;
+ toScale = 2;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(1234568)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 10;
+ toScale = 5;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(1234567890)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 10;
+ toScale = 6;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true);
+
+ fromScale = 5;
+ toPrecision = 11;
+ toScale = 0;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12346));
+
+ fromScale = 5;
+ toPrecision = 11;
+ toScale = 3;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345679)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 11;
+ toScale = 6;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345678900)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 11;
+ toScale = 7;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true); // overflow
+
+ fromScale = 5;
+ toPrecision = 12;
+ toScale = 5;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(1234567890)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 12;
+ toScale = 6;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345678900)) << pair.second.toString();
+
+ fromScale = 5;
+ toPrecision = 12;
+ toScale = 8;
+ pair = convertDecimal(num, fromScale, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true); // overflow
+ }
+
+ TEST(Int128, testConvertDecimaFromFloat) {
+ double fromDouble = 12345.13579;
+ int toPrecision = 4;
+ int toScale = 2;
+ auto pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true); // overflow
+
+ toPrecision = 5;
+ toScale = 0;
+ pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345)) << pair.second.toString();
+
+ toPrecision = 5;
+ toScale = 1;
+ pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true); // overflow
+
+ toPrecision = 6;
+ toScale = 0;
+ pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345)) << pair.second.toString();
+
+ toPrecision = 6;
+ toScale = 1;
+ pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(123451)) << pair.second.toString();
+
+ toPrecision = 6;
+ toScale = 2;
+ pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, true); // overflow
+
+ toPrecision = 8;
+ toScale = 3;
+ pair = convertDecimal(fromDouble, toPrecision, toScale);
+ EXPECT_EQ(pair.first, false); // no overflow
+ EXPECT_EQ(pair.second, Int128(12345136)) << pair.second.toString();
+
+ fromDouble = -12345.13579;
Review Comment:
Add large numbers that are beyond 18 digits.
--
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]
