Benjamin-Philip commented on PR #114: URL: https://github.com/apache/arrow-erlang/pull/114#issuecomment-5151943658
> ## Pull request overview > > Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments. > Suppressed comments (1) > > **src/arrow_buffer.erl:120** > > * `arrow_buffer:size/1` can legally return `0` for empty buffers (e.g. arrays created from `[]` end up with `Buffer#buffer.length = 0`), but the spec currently claims `pos_integer()`. This will likely trigger Dialyzer warnings and is inconsistent with the implementation. > > > ``` > -spec size(Buffer :: arrow_buffer:buffer()) -> pos_integer(). > size(Buffer) -> > Len = Buffer#buffer.length, > Len + arrow_utils:pad_len(Len). > ``` @kou, according to https://arrow.apache.org/docs/format/Columnar.html#buffer-alignment-and-padding, do empty buffers need to be padded? Right now, I'm taking 0 to be a multiple of 64, so I'm not padding. If we do need to pad 0-bit buffers, I'll have to fix that. As of now, Copilot is right. `pos_integer` should be replaced with `non_neg_integer` since a size of 0 is possible. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
