From 07d106c481a532acb106df8f151a75685293ec7c Mon Sep 17 00:00:00 2001 From: Takashi Hashida Date: Tue, 28 Jan 2020 22:33:06 +0900 Subject: [PATCH 1/3] ARROW-7514_make_getvalueoffset_obsolete Make BinaryArray.GetValueOffset obsolete --- csharp/src/Apache.Arrow/Arrays/BinaryArray.cs | 9 ++++++--- csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs | 6 ++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs b/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs index 49c483fc41f..38288b654fa 100644 --- a/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs +++ b/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs @@ -171,6 +171,7 @@ public BinaryArray(IArrowType dataType, int length, public ReadOnlySpan Values => ValueBuffer.Span.CastTo(); [MethodImpl(MethodImplOptions.AggressiveInlining)] + [Obsolete] public int GetValueOffset(int index) { if (index < 0 || index > Length) @@ -193,10 +194,12 @@ public int GetValueLength(int index) public ReadOnlySpan GetBytes(int index) { - var offset = GetValueOffset(index); - var length = GetValueLength(index); + if (index < 0 || index >= Length) + { + throw new ArgumentOutOfRangeException(nameof(index)); + } - return ValueBuffer.Span.Slice(offset, length); + return ValueBuffer.Span.Slice(ValueOffsets[index], GetValueLength(index)); } } diff --git a/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs b/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs index f2a8e58c89f..61e03f719d8 100644 --- a/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs +++ b/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs @@ -46,6 +46,12 @@ public void ThrowsWhenGetValueAndOffsetIndexOutOfBounds() Assert.Equal(2, array.GetValueOffset(2)); Assert.Throws(() => array.GetValueOffset(3)); + Assert.Throws(() => array.ValueOffsets[-1]); + Assert.Equal(0, array.ValueOffsets[0]); + Assert.Equal(1, array.ValueOffsets[1]); + Assert.Equal(2, array.ValueOffsets[2]); + Assert.Throws(() => array.ValueOffsets[3]); + } [Fact] From 92b14c05523315316456bcbcdb82e7b76a552fcc Mon Sep 17 00:00:00 2001 From: Takashi Hashida Date: Tue, 4 Feb 2020 12:06:38 +0900 Subject: [PATCH 2/3] ARROW-7514_make_getvalueoffset_obsolete Respond to feedback Add a message to Obsolete attributes Avoid Obsolete warnings --- csharp/src/Apache.Arrow/Arrays/BinaryArray.cs | 2 +- csharp/src/Apache.Arrow/Arrays/ListArray.cs | 2 +- csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs b/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs index 38288b654fa..3f01b9374f5 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, public ReadOnlySpan Values => ValueBuffer.Span.CastTo(); [MethodImpl(MethodImplOptions.AggressiveInlining)] - [Obsolete] + [Obsolete("This method has been deprecated. Please use ValueOffsets instead.")] public int GetValueOffset(int index) { if (index < 0 || index > Length) diff --git a/csharp/src/Apache.Arrow/Arrays/ListArray.cs b/csharp/src/Apache.Arrow/Arrays/ListArray.cs index ff7095d5da6..0c7c1a4d13a 100644 --- a/csharp/src/Apache.Arrow/Arrays/ListArray.cs +++ b/csharp/src/Apache.Arrow/Arrays/ListArray.cs @@ -123,7 +123,7 @@ private ListArray(ArrayData data, IArrowArray values) : base(data) public override void Accept(IArrowArrayVisitor visitor) => Accept(this, visitor); - [Obsolete] + [Obsolete("This method has been deprecated. Please use ValueOffsets instead.")] public int GetValueOffset(int index) { if (index < 0 || index > Length) diff --git a/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs b/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs index 61e03f719d8..a1a7b231314 100644 --- a/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs +++ b/csharp/test/Apache.Arrow.Tests/ArrowArrayTests.cs @@ -40,11 +40,13 @@ public void ThrowsWhenGetValueAndOffsetIndexOutOfBounds() Assert.Equal(1, array.GetValueLength(1)); Assert.Throws(() => array.GetValueLength(2)); +#pragma warning disable 618 Assert.Throws(() => array.GetValueOffset(-1)); Assert.Equal(0, array.GetValueOffset(0)); Assert.Equal(1, array.GetValueOffset(1)); Assert.Equal(2, array.GetValueOffset(2)); Assert.Throws(() => array.GetValueOffset(3)); +#pragma warning restore 618 Assert.Throws(() => array.ValueOffsets[-1]); Assert.Equal(0, array.ValueOffsets[0]); From 1dbaf39f09bf356d6df4e449781a4abca6b68b78 Mon Sep 17 00:00:00 2001 From: Takashi Hashida Date: Tue, 4 Feb 2020 14:08:26 +0900 Subject: [PATCH 3/3] ARROW-7514_make_getvalueoffset_obsolete Change a message of Obsolete attributes --- csharp/src/Apache.Arrow/Arrays/BinaryArray.cs | 2 +- csharp/src/Apache.Arrow/Arrays/ListArray.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs b/csharp/src/Apache.Arrow/Arrays/BinaryArray.cs index 3f01b9374f5..e2cf0bc772a 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, public ReadOnlySpan Values => ValueBuffer.Span.CastTo(); [MethodImpl(MethodImplOptions.AggressiveInlining)] - [Obsolete("This method has been deprecated. Please use ValueOffsets instead.")] + [Obsolete("This method has been deprecated. Please use ValueOffsets[index] instead.")] public int GetValueOffset(int index) { if (index < 0 || index > Length) diff --git a/csharp/src/Apache.Arrow/Arrays/ListArray.cs b/csharp/src/Apache.Arrow/Arrays/ListArray.cs index 0c7c1a4d13a..bc171e6ffbd 100644 --- a/csharp/src/Apache.Arrow/Arrays/ListArray.cs +++ b/csharp/src/Apache.Arrow/Arrays/ListArray.cs @@ -123,7 +123,7 @@ private ListArray(ArrayData data, IArrowArray values) : base(data) public override void Accept(IArrowArrayVisitor visitor) => Accept(this, visitor); - [Obsolete("This method has been deprecated. Please use ValueOffsets instead.")] + [Obsolete("This method has been deprecated. Please use ValueOffsets[index] instead.")] public int GetValueOffset(int index) { if (index < 0 || index > Length)