From 756e5cfc9941ae15ca817b99e39afe12d861d786 Mon Sep 17 00:00:00 2001 From: takashi hashida Date: Fri, 13 Dec 2019 15:10:27 +0900 Subject: [PATCH 1/4] Fix Values and ValueOffsets start position --- csharp/src/Apache.Arrow/Arrays/BinaryArray.cs | 6 +- csharp/src/Apache.Arrow/Arrays/ListArray.cs | 2 +- .../src/Apache.Arrow/Arrays/PrimitiveArray.cs | 2 +- .../Apache.Arrow.Tests/ArrowArrayTests.cs | 107 ++++++++++++++++++ 4 files changed, 112 insertions(+), 5 deletions(-) create mode 100644 csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs diff --git a/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs b/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs index 4cd82b6154b..2d771abaef2 100644 --- a/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs +++ b/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs @@ -164,21 +164,21 @@ public BinaryArray(IArrowType dataType, int length, public ArrowBuffer ValueBuffer => Data.Buffers[2]; - public ReadOnlySpan ValueOffsets => ValueOffsetsBuffer.Span.CastTo().Slice(0, Length + 1); + public ReadOnlySpan ValueOffsets => ValueOffsetsBuffer.Span.CastTo().Slice(Offset, Length + 1); public ReadOnlySpan Values => ValueBuffer.Span.CastTo(); [MethodImpl(MethodImplOptions.AggressiveInlining)] public int GetValueOffset(int index) { - return ValueOffsets[Offset + index]; + return ValueOffsets[index]; } [MethodImpl(MethodImplOptions.AggressiveInlining)] public int GetValueLength(int index) { var offsets = ValueOffsets; - var offset = Offset + index; + var offset = index; return offsets[offset + 1] - offsets[offset]; } diff --git a/csharp/src/Apache.Arrow/Arrays/ListArray.cs b/csharp/src/Apache.Arrow/Arrays/ListArray.cs index 6b037ce4cd2..3f89586907e 100644 --- a/csharp/src/Apache.Arrow/Arrays/ListArray.cs +++ b/csharp/src/Apache.Arrow/Arrays/ListArray.cs @@ -24,7 +24,7 @@ public class ListArray : Array public ArrowBuffer ValueOffsetsBuffer => Data.Buffers[1]; - public ReadOnlySpan ValueOffsets => ValueOffsetsBuffer.Span.CastTo().Slice(0, Length + 1); + public ReadOnlySpan ValueOffsets => ValueOffsetsBuffer.Span.CastTo().Slice(Offset, Length + 1); public ListArray(IArrowType dataType, int length, ArrowBuffer valueOffsetsBuffer, IArrowArray values, diff --git a/csharp/src/Apache.Arrow/Arrays/PrimitiveArray.cs b/csharp/src/Apache.Arrow/Arrays/PrimitiveArray.cs index 60982664ff7..dbf576b4c69 100644 --- a/csharp/src/Apache.Arrow/Arrays/PrimitiveArray.cs +++ b/csharp/src/Apache.Arrow/Arrays/PrimitiveArray.cs @@ -30,7 +30,7 @@ protected PrimitiveArray(ArrayData data) public ArrowBuffer ValueBuffer => Data.Buffers[1]; - public ReadOnlySpan Values => ValueBuffer.Span.CastTo().Slice(0, Length); + public ReadOnlySpan Values => ValueBuffer.Span.CastTo().Slice(Offset, Length); [MethodImpl(MethodImplOptions.AggressiveInlining)] public T? GetValue(int index) diff --git a/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs b/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs new file mode 100644 index 00000000000..6eabaf96750 --- /dev/null +++ b/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs @@ -0,0 +1,107 @@ +using Apache.Arrow.Tests.Fixtures; +using Apache.Arrow.Types; +using System; +using System.Text; +using Xunit; + +namespace Apache.Arrow.Tests +{ + public class ArrowArrayTests + { + [Fact] + public void SliceArray() + { + TestSlice(x => x.Append(10).Append(20).Append(30)); + TestSlice(x => x.Append(10).Append(20).Append(30)); + TestSlice(x => x.Append(10).Append(20).Append(30)); + TestSlice(x => x.Append(10).Append(20).Append(30)); + TestSlice(x => x.Append(10).Append(20).Append(30)); + TestSlice(x => x.Append(10).Append(20).Append(30)); + TestSlice(x => x.Append(10).Append(20).Append(30)); + TestSlice(x => x.Append(10).Append(20).Append(30)); + TestSlice(x => x.Append(10).Append(20).Append(30)); + TestSlice(x => x.Append(10).Append(20).Append(30)); + TestSlice(x => x.Append("10").Append("20").Append("30")); + } + + private static void TestSlice(Action action) + where TArray : IArrowArray + where TArrayBuilder : IArrowArrayBuilder, new() + { + var builder = new TArrayBuilder(); + action(builder); + var baseArray = builder.Build(default) as Array; + Assert.NotNull(baseArray); + var totalLength = baseArray.Length; + var validator = new ArraySliceValidator(baseArray); + + //Check all offset and length + for (var offset = 0; offset < totalLength; offset++) + { + for(var length = 1; length + offset <= totalLength; length++) + { + var targetArray = baseArray.Slice(offset, length); + targetArray.Accept(validator); + } + } + } + + private class ArraySliceValidator : + IArrowArrayVisitor, + IArrowArrayVisitor, + IArrowArrayVisitor, + IArrowArrayVisitor, + IArrowArrayVisitor, + IArrowArrayVisitor, + IArrowArrayVisitor, + IArrowArrayVisitor, + IArrowArrayVisitor, + IArrowArrayVisitor, + IArrowArrayVisitor + { + private readonly IArrowArray _baseArray; + + public ArraySliceValidator(IArrowArray baseArray) + { + _baseArray = baseArray; + } + + public void Visit(Int8Array array) => ValidateArrays(array); + public void Visit(Int16Array array) => ValidateArrays(array); + public void Visit(Int32Array array) => ValidateArrays(array); + public void Visit(Int64Array array) => ValidateArrays(array); + public void Visit(UInt8Array array) => ValidateArrays(array); + public void Visit(UInt16Array array) => ValidateArrays(array); + public void Visit(UInt32Array array) => ValidateArrays(array); + public void Visit(UInt64Array array) => ValidateArrays(array); + public void Visit(FloatArray array) => ValidateArrays(array); + public void Visit(DoubleArray array) => ValidateArrays(array); + public void Visit(StringArray array) => ValidateArrays(array); + + public void Visit(IArrowArray array) => throw new NotImplementedException(); + + private void ValidateArrays(PrimitiveArray slicedArray) + where T : struct, IEquatable + { + Assert.IsAssignableFrom>(_baseArray); + var baseArray = (PrimitiveArray)_baseArray; + + Assert.True(baseArray.NullBitmapBuffer.Span.SequenceEqual(slicedArray.NullBitmapBuffer.Span)); + Assert.True( + baseArray.ValueBuffer.Span.CastTo().Slice(slicedArray.Offset, slicedArray.Length) + .SequenceEqual(slicedArray.Values)); + } + + private void ValidateArrays(BinaryArray slicedArray) + { + Assert.IsAssignableFrom(_baseArray); + var baseArray = (BinaryArray)_baseArray; + + Assert.True(baseArray.NullBitmapBuffer.Span.SequenceEqual(slicedArray.NullBitmapBuffer.Span)); + Assert.True( + baseArray.ValueOffsetsBuffer.Span.CastTo().Slice(slicedArray.Offset, slicedArray.Length + 1) + .SequenceEqual(slicedArray.ValueOffsets)); + } + } + } +} From 8a8614b9dea0af925ea9d3889f5c113d84f4bff7 Mon Sep 17 00:00:00 2001 From: takashi hashida Date: Mon, 16 Dec 2019 23:06:45 +0900 Subject: [PATCH 2/4] Improve GetValueLength Add a license header Format codes --- csharp/src/Apache.Arrow/Arrays/BinaryArray.cs | 25 ++++++++----------- csharp/src/Apache.Arrow/Arrays/ListArray.cs | 2 +- .../src/Apache.Arrow/Arrays/PrimitiveArray.cs | 2 +- .../Apache.Arrow.Tests/ArrowArrayTests.cs | 22 ++++++++++++---- 4 files changed, 30 insertions(+), 21 deletions(-) diff --git a/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs b/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs index 2d771abaef2..87213aa9470 100644 --- a/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs +++ b/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs @@ -21,7 +21,7 @@ namespace Apache.Arrow { - public class BinaryArray: Array + public class BinaryArray : Array { public class Builder : BuilderBase { @@ -42,15 +42,15 @@ public BinaryArray(ArrayData data) } public BinaryArray(ArrowTypeId typeId, ArrayData data) - : base (data) + : base(data) { data.EnsureDataType(typeId); data.EnsureBufferCount(3); } - public abstract class BuilderBase: IArrowArrayBuilder - where TArray: IArrowArray - where TBuilder: class, IArrowArrayBuilder + public abstract class BuilderBase : IArrowArrayBuilder + where TArray : IArrowArray + where TBuilder : class, IArrowArrayBuilder { protected IArrowType DataType { get; } protected TBuilder Instance => this as TBuilder; @@ -71,8 +71,8 @@ public TArray Build(MemoryAllocator allocator = default) { ValueOffsets.Append(Offset); - var data = new ArrayData(DataType, ValueOffsets.Length - 1, 0, 0, - new [] { ArrowBuffer.Empty, ValueOffsets.Build(allocator), ValueBuffer.Build(allocator) }); + var data = new ArrayData(DataType, ValueOffsets.Length - 1, 0, 0, + new[] { ArrowBuffer.Empty, ValueOffsets.Build(allocator), ValueBuffer.Build(allocator) }); return Build(data); } @@ -154,8 +154,8 @@ public BinaryArray(IArrowType dataType, int length, ArrowBuffer dataBuffer, ArrowBuffer nullBitmapBuffer, int nullCount = 0, int offset = 0) - : this(new ArrayData(dataType, length, nullCount, offset, - new [] { nullBitmapBuffer, valueOffsetsBuffer, dataBuffer })) + : this(new ArrayData(dataType, length, nullCount, offset, + new[] { nullBitmapBuffer, valueOffsetsBuffer, dataBuffer })) { } public override void Accept(IArrowArrayVisitor visitor) => Accept(this, visitor); @@ -177,17 +177,14 @@ public int GetValueOffset(int index) [MethodImpl(MethodImplOptions.AggressiveInlining)] public int GetValueLength(int index) { - var offsets = ValueOffsets; - var offset = index; - - return offsets[offset + 1] - offsets[offset]; + return ValueOffsets[index + 1] - ValueOffsets[index]; } public ReadOnlySpan GetBytes(int index) { var offset = GetValueOffset(index); var length = GetValueLength(index); - + return ValueBuffer.Span.Slice(offset, length); } diff --git a/csharp/src/Apache.Arrow/Arrays/ListArray.cs b/csharp/src/Apache.Arrow/Arrays/ListArray.cs index 3f89586907e..0a03751e21c 100644 --- a/csharp/src/Apache.Arrow/Arrays/ListArray.cs +++ b/csharp/src/Apache.Arrow/Arrays/ListArray.cs @@ -30,7 +30,7 @@ public ListArray(IArrowType dataType, int length, ArrowBuffer valueOffsetsBuffer, IArrowArray values, ArrowBuffer nullBitmapBuffer, int nullCount = 0, int offset = 0) : this(new ArrayData(dataType, length, nullCount, offset, - new[] {nullBitmapBuffer, valueOffsetsBuffer}, new[] {values.Data})) + new[] { nullBitmapBuffer, valueOffsetsBuffer }, new[] { values.Data })) { Values = values; } diff --git a/csharp/src/Apache.Arrow/Arrays/PrimitiveArray.cs b/csharp/src/Apache.Arrow/Arrays/PrimitiveArray.cs index dbf576b4c69..8bb27559f91 100644 --- a/csharp/src/Apache.Arrow/Arrays/PrimitiveArray.cs +++ b/csharp/src/Apache.Arrow/Arrays/PrimitiveArray.cs @@ -35,7 +35,7 @@ protected PrimitiveArray(ArrayData data) [MethodImpl(MethodImplOptions.AggressiveInlining)] public T? GetValue(int index) { - return IsValid(index) ? Values[index] : (T?) null; + return IsValid(index) ? Values[index] : (T?)null; } public IList ToList(bool includeNulls = false) diff --git a/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs b/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs index 6eabaf96750..f9e462f3a0f 100644 --- a/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs +++ b/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs @@ -1,7 +1,19 @@ -using Apache.Arrow.Tests.Fixtures; -using Apache.Arrow.Types; +// 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; -using System.Text; using Xunit; namespace Apache.Arrow.Tests @@ -38,14 +50,14 @@ private static void TestSlice(Action actio //Check all offset and length for (var offset = 0; offset < totalLength; offset++) { - for(var length = 1; length + offset <= totalLength; length++) + for (var length = 1; length + offset <= totalLength; length++) { var targetArray = baseArray.Slice(offset, length); targetArray.Accept(validator); } } } - + private class ArraySliceValidator : IArrowArrayVisitor, IArrowArrayVisitor, From d012ba5348efccd36a23d0f3b3100b301db01051 Mon Sep 17 00:00:00 2001 From: takashi hashida Date: Thu, 19 Dec 2019 10:43:33 +0900 Subject: [PATCH 3/4] Fix IsValid Fix GetDate Add boundary check to GetValue and so on Improve tests --- csharp/src/Apache.Arrow/Arrays/Array.cs | 2 +- csharp/src/Apache.Arrow/Arrays/BinaryArray.cs | 11 ++- csharp/src/Apache.Arrow/Arrays/Date32Array.cs | 14 +-- csharp/src/Apache.Arrow/Arrays/Date64Array.cs | 13 +-- .../src/Apache.Arrow/Arrays/PrimitiveArray.cs | 4 + .../Apache.Arrow.Tests/ArrowArrayTests.cs | 91 +++++++++++++++++++ 6 files changed, 117 insertions(+), 18 deletions(-) diff --git a/csharp/src/Apache.Arrow/Arrays/Array.cs b/csharp/src/Apache.Arrow/Arrays/Array.cs index 77794ab2865..a453b080726 100644 --- a/csharp/src/Apache.Arrow/Arrays/Array.cs +++ b/csharp/src/Apache.Arrow/Arrays/Array.cs @@ -41,7 +41,7 @@ public virtual void Accept(IArrowArrayVisitor visitor) } public bool IsValid(int index) => - NullCount == 0 || NullBitmapBuffer.IsEmpty || BitUtility.GetBit(NullBitmapBuffer.Span, index); + NullCount == 0 || NullBitmapBuffer.IsEmpty || BitUtility.GetBit(NullBitmapBuffer.Span, index + Offset); public bool IsNull(int index) => !IsValid(index); diff --git a/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs b/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs index 87213aa9470..16cbd8b8a12 100644 --- a/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs +++ b/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs @@ -171,13 +171,22 @@ public BinaryArray(IArrowType dataType, int length, [MethodImpl(MethodImplOptions.AggressiveInlining)] public int GetValueOffset(int index) { + if (index < 0 || index >= Length) + { + throw new ArgumentOutOfRangeException(nameof(index)); + } return ValueOffsets[index]; } [MethodImpl(MethodImplOptions.AggressiveInlining)] public int GetValueLength(int index) { - return ValueOffsets[index + 1] - ValueOffsets[index]; + if (index < 0 || index >= Length) + { + throw new ArgumentOutOfRangeException(nameof(index)); + } + var offsets = ValueOffsets; + return offsets[index + 1] - offsets[index]; } public ReadOnlySpan GetBytes(int index) diff --git a/csharp/src/Apache.Arrow/Arrays/Date32Array.cs b/csharp/src/Apache.Arrow/Arrays/Date32Array.cs index bc9c35146ca..bb87fbf6b5d 100644 --- a/csharp/src/Apache.Arrow/Arrays/Date32Array.cs +++ b/csharp/src/Apache.Arrow/Arrays/Date32Array.cs @@ -18,7 +18,7 @@ namespace Apache.Arrow { - public class Date32Array: PrimitiveArray + public class Date32Array : PrimitiveArray { private const int MillisecondsPerDay = 86400000; @@ -36,7 +36,7 @@ public Builder() : base(new DateBuilder()) { } protected override int ConvertTo(DateTimeOffset value) { - return (int) (value.ToUnixTimeMilliseconds() / MillisecondsPerDay); + return (int)(value.ToUnixTimeMilliseconds() / MillisecondsPerDay); } } @@ -47,7 +47,7 @@ public Date32Array( new[] { nullBitmapBuffer, valueBuffer })) { } - public Date32Array(ArrayData data) + public Date32Array(ArrayData data) : base(data) { data.EnsureDataType(ArrowTypeId.Date32); @@ -57,10 +57,10 @@ public Date32Array(ArrayData data) public DateTimeOffset? GetDate(int index) { - var values = ValueBuffer.Span.CastTo().Slice(0, Length); - var value = values[index]; - - return DateTimeOffset.FromUnixTimeMilliseconds(value * MillisecondsPerDay); + var value = GetValue(index); + return value.HasValue ? + DateTimeOffset.FromUnixTimeMilliseconds(value.Value * MillisecondsPerDay) : + null as DateTimeOffset?; } } } diff --git a/csharp/src/Apache.Arrow/Arrays/Date64Array.cs b/csharp/src/Apache.Arrow/Arrays/Date64Array.cs index bf1e15ce756..c71efb18f67 100644 --- a/csharp/src/Apache.Arrow/Arrays/Date64Array.cs +++ b/csharp/src/Apache.Arrow/Arrays/Date64Array.cs @@ -57,15 +57,10 @@ public Date64Array(ArrayData data) public DateTimeOffset? GetDate(int index) { - if (!IsValid(index)) - { - return null; - } - - var values = ValueBuffer.Span.CastTo().Slice(0, Length); - var value = values[index]; - - return DateTimeOffset.FromUnixTimeMilliseconds(value); + var value = GetValue(index); + return value.HasValue ? + DateTimeOffset.FromUnixTimeMilliseconds(value.Value) : + null as DateTimeOffset?; } } } diff --git a/csharp/src/Apache.Arrow/Arrays/PrimitiveArray.cs b/csharp/src/Apache.Arrow/Arrays/PrimitiveArray.cs index 8bb27559f91..d44a352309d 100644 --- a/csharp/src/Apache.Arrow/Arrays/PrimitiveArray.cs +++ b/csharp/src/Apache.Arrow/Arrays/PrimitiveArray.cs @@ -35,6 +35,10 @@ protected PrimitiveArray(ArrayData data) [MethodImpl(MethodImplOptions.AggressiveInlining)] public T? GetValue(int index) { + if (index < 0 || index >= Length) + { + throw new ArgumentOutOfRangeException(nameof(index)); + } return IsValid(index) ? Values[index] : (T?)null; } diff --git a/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs b/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs index f9e462f3a0f..449997d8538 100644 --- a/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs +++ b/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs @@ -20,6 +20,69 @@ namespace Apache.Arrow.Tests { public class ArrowArrayTests { + + [Fact] + public void ThrowsWhenGetValueIndexOutOfBounds() + { + var array = new Int64Array.Builder().Append(1).Append(2).Build(); + Assert.Throws(() => array.GetValue(-1)); + Assert.Equal(1, array.GetValue(0)); + Assert.Equal(2, array.GetValue(1)); + Assert.Throws(() => array.GetValue(2)); + } + + [Fact] + public void ThrowsWhenGetValueAndOffsetIndexOutOfBounds() + { + var array = new BinaryArray.Builder().Append(1).Append(2).Build(); + Assert.Throws(() => array.GetValueLength(-1)); + Assert.Equal(1, array.GetValueLength(0)); + Assert.Equal(1, array.GetValueLength(1)); + Assert.Throws(() => array.GetValueLength(2)); + + Assert.Throws(() => array.GetValueOffset(-1)); + Assert.Equal(0, array.GetValueOffset(0)); + Assert.Equal(1, array.GetValueOffset(1)); + Assert.Throws(() => array.GetValueOffset(2)); + + } + + [Fact] + public void IsValidValue() + { + const int totalValueCount = 8; + const byte nullBitmap = 0b_11110011; + + var nullBitmapBuffer = new ArrowBuffer.Builder().Append(nullBitmap).Build(); + var valueBuffer = new ArrowBuffer.Builder().Append(0).Append(1).Append(4).Append(5).Append(6).Append(7).Append(8).Build(); + + //Check all offset and length + for (var offset = 0; offset < totalValueCount; offset++) + { + var nullCount = totalValueCount - offset - BitUtility.CountBits(nullBitmapBuffer.Span, offset); + for (var length = 1; length + offset < totalValueCount; length++) + { + TestIsValid(valueBuffer, nullBitmapBuffer, length, nullCount, offset); + } + } + + void TestIsValid(ArrowBuffer valueBuf, ArrowBuffer nullBitmapBuf, int length, int nullCount, int offset) + { + var array = new Int64Array(valueBuf, nullBitmapBuf, length, nullCount, offset); + for (var i = 0; i < length; i++) + { + if (BitUtility.GetBit(nullBitmap, i + offset)) + { + Assert.True(array.IsValid(i)); + } + else + { + Assert.False(array.IsValid(i)); + } + } + } + } + [Fact] public void SliceArray() { @@ -33,6 +96,8 @@ public void SliceArray() TestSlice(x => x.Append(10).Append(20).Append(30)); TestSlice(x => x.Append(10).Append(20).Append(30)); TestSlice(x => x.Append(10).Append(20).Append(30)); + TestSlice(x => x.Append(new DateTime(2019, 1, 1)).Append(new DateTime(2019, 1, 2)).Append(new DateTime(2019, 1, 3))); + TestSlice(x => x.Append(new DateTime(2019, 1, 1)).Append(new DateTime(2019, 1, 2)).Append(new DateTime(2019, 1, 3))); TestSlice(x => x.Append("10").Append("20").Append("30")); } @@ -67,6 +132,8 @@ private class ArraySliceValidator : IArrowArrayVisitor, IArrowArrayVisitor, IArrowArrayVisitor, + IArrowArrayVisitor, + IArrowArrayVisitor, IArrowArrayVisitor, IArrowArrayVisitor, IArrowArrayVisitor @@ -86,6 +153,25 @@ public ArraySliceValidator(IArrowArray baseArray) public void Visit(UInt16Array array) => ValidateArrays(array); public void Visit(UInt32Array array) => ValidateArrays(array); public void Visit(UInt64Array array) => ValidateArrays(array); + + public void Visit(Date32Array array) + { + ValidateArrays(array); + Assert.IsAssignableFrom(_baseArray); + var baseArray = (Date32Array)_baseArray; + + Assert.Equal(baseArray.GetDate(array.Offset), array.GetDate(0)); + } + + public void Visit(Date64Array array) + { + ValidateArrays(array); + Assert.IsAssignableFrom(_baseArray); + var baseArray = (Date64Array)_baseArray; + + Assert.Equal(baseArray.GetDate(array.Offset), array.GetDate(0)); + } + public void Visit(FloatArray array) => ValidateArrays(array); public void Visit(DoubleArray array) => ValidateArrays(array); public void Visit(StringArray array) => ValidateArrays(array); @@ -102,6 +188,8 @@ private void ValidateArrays(PrimitiveArray slicedArray) Assert.True( baseArray.ValueBuffer.Span.CastTo().Slice(slicedArray.Offset, slicedArray.Length) .SequenceEqual(slicedArray.Values)); + + Assert.Equal(baseArray.GetValue(slicedArray.Offset), slicedArray.GetValue(0)); } private void ValidateArrays(BinaryArray slicedArray) @@ -109,10 +197,13 @@ private void ValidateArrays(BinaryArray slicedArray) Assert.IsAssignableFrom(_baseArray); var baseArray = (BinaryArray)_baseArray; + Assert.True(baseArray.Values.SequenceEqual(slicedArray.Values)); Assert.True(baseArray.NullBitmapBuffer.Span.SequenceEqual(slicedArray.NullBitmapBuffer.Span)); Assert.True( baseArray.ValueOffsetsBuffer.Span.CastTo().Slice(slicedArray.Offset, slicedArray.Length + 1) .SequenceEqual(slicedArray.ValueOffsets)); + + Assert.True(baseArray.GetBytes(slicedArray.Offset).SequenceEqual(slicedArray.GetBytes(0))); } } } From 89f2212fdef48d9019e4e79325bd7ef9714ebd09 Mon Sep 17 00:00:00 2001 From: takashi hashida Date: Fri, 20 Dec 2019 04:05:46 +0900 Subject: [PATCH 4/4] Fix GetValueOffset boundary check Refactor GetDate --- csharp/src/Apache.Arrow/Arrays/BinaryArray.cs | 2 +- csharp/src/Apache.Arrow/Arrays/Date32Array.cs | 10 +++++++--- csharp/src/Apache.Arrow/Arrays/Date64Array.cs | 10 +++++++--- csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs | 3 ++- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs b/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs index 16cbd8b8a12..ece19cfe7f1 100644 --- a/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs +++ b/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs @@ -171,7 +171,7 @@ public BinaryArray(IArrowType dataType, int length, [MethodImpl(MethodImplOptions.AggressiveInlining)] public int GetValueOffset(int index) { - if (index < 0 || index >= Length) + if (index < 0 || index > Length) { throw new ArgumentOutOfRangeException(nameof(index)); } diff --git a/csharp/src/Apache.Arrow/Arrays/Date32Array.cs b/csharp/src/Apache.Arrow/Arrays/Date32Array.cs index bb87fbf6b5d..5099e4074b4 100644 --- a/csharp/src/Apache.Arrow/Arrays/Date32Array.cs +++ b/csharp/src/Apache.Arrow/Arrays/Date32Array.cs @@ -58,9 +58,13 @@ public Date32Array(ArrayData data) public DateTimeOffset? GetDate(int index) { var value = GetValue(index); - return value.HasValue ? - DateTimeOffset.FromUnixTimeMilliseconds(value.Value * MillisecondsPerDay) : - null as DateTimeOffset?; + + if (!value.HasValue) + { + return default; + } + + return DateTimeOffset.FromUnixTimeMilliseconds(value.Value * MillisecondsPerDay); } } } diff --git a/csharp/src/Apache.Arrow/Arrays/Date64Array.cs b/csharp/src/Apache.Arrow/Arrays/Date64Array.cs index c71efb18f67..6b3086c2fbb 100644 --- a/csharp/src/Apache.Arrow/Arrays/Date64Array.cs +++ b/csharp/src/Apache.Arrow/Arrays/Date64Array.cs @@ -58,9 +58,13 @@ public Date64Array(ArrayData data) public DateTimeOffset? GetDate(int index) { var value = GetValue(index); - return value.HasValue ? - DateTimeOffset.FromUnixTimeMilliseconds(value.Value) : - null as DateTimeOffset?; + + if (!value.HasValue) + { + return default; + } + + return DateTimeOffset.FromUnixTimeMilliseconds(value.Value); } } } diff --git a/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs b/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs index 449997d8538..f2a8e58c89f 100644 --- a/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs +++ b/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs @@ -43,7 +43,8 @@ public void ThrowsWhenGetValueAndOffsetIndexOutOfBounds() Assert.Throws(() => array.GetValueOffset(-1)); Assert.Equal(0, array.GetValueOffset(0)); Assert.Equal(1, array.GetValueOffset(1)); - Assert.Throws(() => array.GetValueOffset(2)); + Assert.Equal(2, array.GetValueOffset(2)); + Assert.Throws(() => array.GetValueOffset(3)); }