Skip to content

Commit

Permalink
GH-41199: [C#] Fix accessing values of a sliced decimal array (#41200)
Browse files Browse the repository at this point in the history
### Rationale for this change

Fixes a bug where getting values from a sliced `Decimal128Array` or `Decimal256Array` would return the wrong values.

### What changes are included in this PR?

Updates the various `Get*` methods in `Decimal128Array` and `Decimal256Array` to account for the array offset.

### Are these changes tested?

Yes, I've added new unit tests.

### Are there any user-facing changes?

Yes, this is a user-facing bug fix.
* GitHub Issue: #41199

Authored-by: Adam Reeve <adreeve@gmail.com>
Signed-off-by: Curt Hagenlocher <curt@hagenlocher.org>
  • Loading branch information
adamreeve authored Apr 15, 2024
1 parent 0b33c65 commit fec096a
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 7 deletions.
6 changes: 3 additions & 3 deletions csharp/src/Apache.Arrow/Arrays/Decimal128Array.cs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public Decimal128Array(ArrayData data)
{
return null;
}
return DecimalUtility.GetDecimal(ValueBuffer, index, Scale, ByteWidth);
return DecimalUtility.GetDecimal(ValueBuffer, Offset + index, Scale, ByteWidth);
}

public IList<decimal?> ToList(bool includeNulls = false)
Expand Down Expand Up @@ -177,7 +177,7 @@ public string GetString(int index)
{
return null;
}
return DecimalUtility.GetString(ValueBuffer, index, Precision, Scale, ByteWidth);
return DecimalUtility.GetString(ValueBuffer, Offset + index, Precision, Scale, ByteWidth);
}

public SqlDecimal? GetSqlDecimal(int index)
Expand All @@ -187,7 +187,7 @@ public string GetString(int index)
return null;
}

return DecimalUtility.GetSqlDecimal128(ValueBuffer, index, Precision, Scale);
return DecimalUtility.GetSqlDecimal128(ValueBuffer, Offset + index, Precision, Scale);
}

int IReadOnlyCollection<SqlDecimal?>.Count => Length;
Expand Down
8 changes: 4 additions & 4 deletions csharp/src/Apache.Arrow/Arrays/Decimal256Array.cs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ public Decimal256Array(ArrayData data)
return null;
}

return DecimalUtility.GetDecimal(ValueBuffer, index, Scale, ByteWidth);
return DecimalUtility.GetDecimal(ValueBuffer, Offset + index, Scale, ByteWidth);
}

public IList<decimal?> ToList(bool includeNulls = false)
Expand Down Expand Up @@ -184,7 +184,7 @@ public string GetString(int index)
{
return null;
}
return DecimalUtility.GetString(ValueBuffer, index, Precision, Scale, ByteWidth);
return DecimalUtility.GetString(ValueBuffer, Offset + index, Precision, Scale, ByteWidth);
}

public bool TryGetSqlDecimal(int index, out SqlDecimal? value)
Expand All @@ -196,11 +196,11 @@ public bool TryGetSqlDecimal(int index, out SqlDecimal? value)
}

const int longWidth = 4;
var span = ValueBuffer.Span.CastTo<long>().Slice(index * longWidth);
var span = ValueBuffer.Span.CastTo<long>().Slice((Offset + index) * longWidth);
if ((span[2] == 0 && span[3] == 0) ||
(span[2] == -1 && span[3] == -1))
{
value = DecimalUtility.GetSqlDecimal128(ValueBuffer, 2 * index, Precision, Scale);
value = DecimalUtility.GetSqlDecimal128(ValueBuffer, 2 * (Offset + index), Precision, Scale);
return true;
}

Expand Down
43 changes: 43 additions & 0 deletions csharp/test/Apache.Arrow.Tests/Decimal128ArrayTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -458,5 +458,48 @@ public void AppendRangeSqlDecimal()
}
}
}

[Fact]
public void SliceDecimal128Array()
{
// Arrange
const int originalLength = 50;
const int offset = 3;
const int sliceLength = 32;

var builder = new Decimal128Array.Builder(new Decimal128Type(14, 10));
var random = new Random();

for (int i = 0; i < originalLength; i++)
{
if (random.NextDouble() < 0.2)
{
builder.AppendNull();
}
else
{
builder.Append(i * (decimal)Math.Round(random.NextDouble(), 10));
}
}

var array = builder.Build();

// Act
var slice = (Decimal128Array)array.Slice(offset, sliceLength);

// Assert
Assert.NotNull(slice);
Assert.Equal(sliceLength, slice.Length);
for (int i = 0; i < sliceLength; ++i)
{
Assert.Equal(array.GetValue(offset + i), slice.GetValue(i));
Assert.Equal(array.GetSqlDecimal(offset + i), slice.GetSqlDecimal(i));
Assert.Equal(array.GetString(offset + i), slice.GetString(i));
}

Assert.Equal(
array.ToList(includeNulls: true).Skip(offset).Take(sliceLength).ToList(),
slice.ToList(includeNulls: true));
}
}
}
51 changes: 51 additions & 0 deletions csharp/test/Apache.Arrow.Tests/Decimal256ArrayTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -476,5 +476,56 @@ public void AppendRangeSqlDecimal()
}
}
}

[Fact]
public void SliceDecimal256Array()
{
// Arrange
const int originalLength = 50;
const int offset = 3;
const int sliceLength = 32;

var builder = new Decimal256Array.Builder(new Decimal256Type(14, 10));
var random = new Random();

for (int i = 0; i < originalLength; i++)
{
if (random.NextDouble() < 0.2)
{
builder.AppendNull();
}
else
{
builder.Append(i * (decimal)Math.Round(random.NextDouble(), 10));
}
}

var array = builder.Build();

// Act
var slice = (Decimal256Array)array.Slice(offset, sliceLength);

// Assert
Assert.NotNull(slice);
Assert.Equal(sliceLength, slice.Length);
for (int i = 0; i < sliceLength; ++i)
{
Assert.Equal(array.GetValue(offset + i), slice.GetValue(i));
if (array.TryGetSqlDecimal(offset + i, out var expectedSqlDecimal))
{
Assert.True(slice.TryGetSqlDecimal(i, out var actualSqlDecimal));
Assert.Equal(expectedSqlDecimal, actualSqlDecimal);
}
else
{
Assert.False(slice.TryGetSqlDecimal(i, out _));
}
Assert.Equal(array.GetString(offset + i), slice.GetString(i));
}

Assert.Equal(
array.ToList(includeNulls: true).Skip(offset).Take(sliceLength).ToList(),
slice.ToList(includeNulls: true));
}
}
}

0 comments on commit fec096a

Please sign in to comment.