mr-smidge commented on a change in pull request #7158: URL: https://github.com/apache/arrow/pull/7158#discussion_r428559607
########## File path: csharp/src/Apache.Arrow/ArrowBuffer.Builder.cs ########## @@ -80,27 +140,59 @@ public Builder<T> AppendRange(IEnumerable<T> values) return this; } - public Builder<T> Reserve(int capacity) + /// <summary> + /// Reserve a given number of items' additional capacity. + /// </summary> + /// <param name="additionalCapacity">Number of items of required additional capacity.</param> + /// <returns>Returns the builder (for fluent-style composition).</returns> + public Builder<T> Reserve(int additionalCapacity) { - EnsureCapacity(capacity); + if (additionalCapacity < 0) + { + throw new ArgumentOutOfRangeException(nameof(additionalCapacity)); + } + + EnsureAdditionalCapacity(additionalCapacity); return this; } + /// <summary> + /// Resize the buffer to a given size. + /// </summary> + /// <remarks> + /// Note that if the required capacity is smaller than the current length of the populated buffer so far, + /// the buffer will be truncated and items at the end of the buffer will be lost. + /// </remarks> + /// <remarks> + /// Note also that a negative capacity will result in the buffer being resized to zero. + /// </remarks> + /// <param name="capacity">Number of items of required capacity.</param> + /// <returns>Returns the builder (for fluent-style composition).</returns> public Builder<T> Resize(int capacity) { + capacity = capacity < 0 ? 0 : capacity; Review comment: If it were me, I would throw `ArgumentOutOfRangeException` if I received a request to resize to a negative length, as arrays can't have negative length. I can't imagine why someone would have written a unit test for the current behaviour. Incidentally, this is what existing Array-resizing methods already do: https://docs.microsoft.com/en-us/dotnet/api/system.array.resize?view=netcore-3.1 And the C++ implementation also refuses to accept negative lengths: https://github.com/apache/arrow/blob/master/cpp/src/arrow/array/builder_base.h#L194 ---------------------------------------------------------------- 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