Skip to content

Commit

Permalink
GH-41137: [C#] Fix DenseUnionArray IsNull/Valid (#41138)
Browse files Browse the repository at this point in the history
### Rationale for this change

Fixes incorrect logic for IsNull and IsValid on DenseUnionArrays

### What changes are included in this PR?

Adds a new abstract `FieldIsValid` method to `UnionArray` to customize behaviour for sparse and dense union arrays.

### Are these changes tested?

Yes, I've added unit tests.

### Are there any user-facing changes?

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

Authored-by: Adam Reeve <adreeve@gmail.com>
Signed-off-by: Curt Hagenlocher <curt@hagenlocher.org>
  • Loading branch information
adamreeve authored and raulcd committed Apr 29, 2024
1 parent ee37dd2 commit 814bb5e
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 1 deletion.
5 changes: 5 additions & 0 deletions csharp/src/Apache.Arrow/Arrays/DenseUnionArray.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,5 +48,10 @@ public DenseUnionArray(ArrayData data)
ValidateMode(UnionMode.Dense, Type.Mode);
data.EnsureBufferCount(2);
}

protected override bool FieldIsValid(IArrowArray fieldArray, int index)
{
return fieldArray.IsValid(ValueOffsets[index]);
}
}
}
5 changes: 5 additions & 0 deletions csharp/src/Apache.Arrow/Arrays/SparseUnionArray.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,10 @@ public SparseUnionArray(ArrayData data)
ValidateMode(UnionMode.Sparse, Type.Mode);
data.EnsureBufferCount(1);
}

protected override bool FieldIsValid(IArrowArray fieldArray, int index)
{
return fieldArray.IsValid(index);
}
}
}
4 changes: 3 additions & 1 deletion csharp/src/Apache.Arrow/Arrays/UnionArray.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public abstract class UnionArray : IArrowArray

public int NullCount => Data.NullCount;

public bool IsValid(int index) => NullCount == 0 || Fields[TypeIds[index]].IsValid(index);
public bool IsValid(int index) => NullCount == 0 || FieldIsValid(Fields[TypeIds[index]], index);

public bool IsNull(int index) => !IsValid(index);

Expand All @@ -65,6 +65,8 @@ public static UnionArray Create(ArrayData data)

public void Accept(IArrowArrayVisitor visitor) => Array.Accept(this, visitor);

protected abstract bool FieldIsValid(IArrowArray field, int index);

public void Dispose()
{
Dispose(true);
Expand Down
113 changes: 113 additions & 0 deletions csharp/test/Apache.Arrow.Tests/UnionArrayTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
// Licensed to the Apache Software Foundation (ASF) under one or more
// contributor license agreements. See the NOTICE file distributed with
// this work for additional information regarding copyright ownership.
// The ASF licenses this file to You under the Apache License, Version 2.0
// (the "License"); you may not use this file except in compliance with
// the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

using System.Linq;
using Apache.Arrow.Types;
using Xunit;

namespace Apache.Arrow.Tests;

public class UnionArrayTests
{
[Theory]
[InlineData(UnionMode.Sparse)]
[InlineData(UnionMode.Dense)]
public void UnionArray_IsNull(UnionMode mode)
{
var fields = new Field[]
{
new Field("field0", new Int32Type(), true),
new Field("field1", new FloatType(), true),
};
var typeIds = fields.Select(f => (int) f.DataType.TypeId).ToArray();
var type = new UnionType(fields, typeIds, mode);

const int length = 100;
var nullCount = 0;
var field0Builder = new Int32Array.Builder();
var field1Builder = new FloatArray.Builder();
var typeIdsBuilder = new ArrowBuffer.Builder<byte>();
var valuesOffsetBuilder = new ArrowBuffer.Builder<int>();
var expectedNull = new bool[length];

for (var i = 0; i < length; ++i)
{
var isNull = i % 5 == 0;
expectedNull[i] = isNull;
nullCount += isNull ? 1 : 0;

if (i % 2 == 0)
{
typeIdsBuilder.Append(0);
if (mode == UnionMode.Sparse)
{
field1Builder.Append(0.0f);
}
else
{
valuesOffsetBuilder.Append(field0Builder.Length);
}

if (isNull)
{
field0Builder.AppendNull();
}
else
{
field0Builder.Append(i);
}
}
else
{
typeIdsBuilder.Append(1);
if (mode == UnionMode.Sparse)
{
field0Builder.Append(0);
}
else
{
valuesOffsetBuilder.Append(field1Builder.Length);
}

if (isNull)
{
field1Builder.AppendNull();
}
else
{
field1Builder.Append(i * 0.1f);
}
}
}

var typeIdsBuffer = typeIdsBuilder.Build();
var valuesOffsetBuffer = valuesOffsetBuilder.Build();
var children = new IArrowArray[]
{
field0Builder.Build(),
field1Builder.Build()
};

UnionArray array = mode == UnionMode.Dense
? new DenseUnionArray(type, length, children, typeIdsBuffer, valuesOffsetBuffer, nullCount)
: new SparseUnionArray(type, length, children, typeIdsBuffer, nullCount);

for (var i = 0; i < length; ++i)
{
Assert.Equal(expectedNull[i], array.IsNull(i));
Assert.Equal(!expectedNull[i], array.IsValid(i));
}
}
}

0 comments on commit 814bb5e

Please sign in to comment.