mr-smidge commented on a change in pull request #7671: URL: https://github.com/apache/arrow/pull/7671#discussion_r451216615
########## File path: csharp/src/Apache.Arrow/Arrays/BinaryArray.cs ########## @@ -237,7 +329,9 @@ public ReadOnlySpan<byte> GetBytes(int index) if (IsNull(index)) { - return null; + // Note that `return null;` is valid syntax, but would be misleading as `null` in the context of a span + // is actually returned as an empty span. + return ReadOnlySpan<byte>.Empty; Review comment: This clause previously said `return null;`, which was misleading as it suggested that the method could be used to identify null values in the array. However, `null` in the context of a `ReadOnlySpan<T>` is the same as the empty span, and so the method can't distinguish null values from empty ones: ```csharp Assert.True(ReadOnlySpan<byte>.Empty == null) ``` The new code is clearer in intent. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org