zeroshade commented on a change in pull request #12278:
URL: https://github.com/apache/arrow/pull/12278#discussion_r793915230
##########
File path: go/arrow/ipc/writer.go
##########
@@ -587,16 +587,21 @@ func (w *recordEncoder) encodeMetadata(p *Payload, nrows
int64) error {
}
func newTruncatedBitmap(mem memory.Allocator, offset, length int64, input
*memory.Buffer) *memory.Buffer {
- if input != nil {
- input.Retain()
- return input
+ if input == nil {
+ return nil
}
minLength := paddedLength(bitutil.BytesForBits(length), kArrowAlignment)
switch {
case offset != 0 || minLength < int64(input.Len()):
// with a sliced array / non-zero offset, we must copy the
bitmap
- panic("not implemented") // FIXME(sbinet): writer.cc:75
+ buf := memory.NewResizableBuffer(mem)
+ buf.Resize(int(minLength))
+ // only copy if some bits are set since CopyBitmap is slow
+ if bitutil.CountSetBits(input.Bytes(), int(offset),
int(length)) > 0 {
+ bitutil.CopyBitmap(input.Bytes(), int(offset),
int(length), buf.Bytes(), 0)
+ }
Review comment:
I'd rather check `data.NullN() == data.Len()` above this function, it's
cheaper and faster than `CountSetBits` above this function you can do something
like:
```go
if data.NullN() == data.Len() {
bitmap = memory.NewResizableBuffer(mem)
bitmap.Resize(int(paddedLength(bitutil.BytesForBits(data.Len()),
kArrowAlignment)))
} else {
bitmap = newTruncatedBitmap(.....)
}
```
Though that wouldn't work for the boolean case since the 0s aren't nulls,
they are just falses. I dunno. Thoughts?
--
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]
