eerhardt commented on code in PR #13732:
URL: https://github.com/apache/arrow/pull/13732#discussion_r932336495
##########
csharp/src/Apache.Arrow/DecimalUtility.cs:
##########
@@ -99,7 +99,7 @@ internal static void GetBytes(decimal value, int precision,
int scale, int byteW
}
bigInt = new BigInteger(bigIntBytes);
#else
- byte[] bigIntBytes = new byte[12];
+ byte[] bigIntBytes = new byte[13];
Review Comment:
(nit) while we are in here, can we fix the existing whitespace issue?
```suggestion
byte[] bigIntBytes = new byte[13];
```
##########
csharp/test/Apache.Arrow.Tests/Decimal128ArrayTests.cs:
##########
@@ -97,6 +97,22 @@ public void AppendLargeDecimal()
Assert.Equal(-large, array.GetValue(1));
}
+ [Fact]
+ public void AppendMaxAndMinDecimal()
+ {
+ // Assert
+ var builder = new Decimal128Array.Builder(new
Decimal128Type(29, 0));
+
+ // Act
+ builder.Append(Decimal.MaxValue);
Review Comment:
I don't think it would hurt to also test a value slightly less than max, and
slightly more than min as well.
```suggestion
builder.Append(Decimal.MaxValue);
builder.Append(Decimal.MaxValue - 10);
builder.Append(Decimal.MinValue + 10);
```
##########
csharp/test/Apache.Arrow.Tests/Decimal256ArrayTests.cs:
##########
@@ -97,6 +97,22 @@ public void AppendLargeDecimal()
Assert.Equal(-large, array.GetValue(1));
}
+ [Fact]
+ public void AppendMaxAndMinDecimal()
+ {
+ // Assert
+ var builder = new Decimal128Array.Builder(new
Decimal128Type(29, 0));
Review Comment:
This is the Decimal256 tests, but we are using Decimal128 here
```suggestion
var builder = new Decimal256Array.Builder(new
Decimal128Type(29, 0));
```
--
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]
