kou commented on code in PR #50452:
URL: https://github.com/apache/arrow/pull/50452#discussion_r3556406529
##########
ruby/red-arrow-format/lib/arrow-format/array.rb:
##########
@@ -537,7 +537,17 @@ class DurationArray < TemporalArray
end
class VariableSizeBinaryArray < Array
- def initialize(size, validity_buffer, offsets_buffer, values_buffer)
+ include BufferAlignable
Review Comment:
Could you add an empty line after `include`?
```suggestion
include BufferAlignable
```
##########
ruby/red-arrow-format/lib/arrow-format/array.rb:
##########
@@ -577,6 +587,48 @@ def to_a
end
private
+ def build_data(data)
+ type = self.class.type
+
+ n = 0
+ validity_buffer_builder = nil
+
+ values = +"".b
+ offsets = [0]
+
+ data.each_with_index do |value, i|
+ if value.nil?
+ validity_buffer_builder ||= SparseBitmapBuilder.new
+ validity_buffer_builder.unset(i)
+
+ offsets << values.bytesize
+ else
+ values.append_as_bytes(value)
+ offsets << values.bytesize
+ end
+
+ n += 1
+ end
+
+ validity_buffer = validity_buffer_builder&.finish(n)
+
+ pad!(values, buffer_padding_size(values))
+ values.freeze
+ offsets_data = offsets.pack("#{type.offset_pack_template}*")
+ pad!(offsets_data, buffer_padding_size(offsets_data))
+ offsets_data.freeze
+ offsets_buffer = IO::Buffer.for(offsets_data)
+
+ values_buffer = IO::Buffer.for(values)
Review Comment:
Let's group offsets related code and values related code by an empty line:
```suggestion
offsets_data = offsets.pack("#{type.offset_pack_template}*")
pad!(offsets_data, buffer_padding_size(offsets_data))
offsets_data.freeze
offsets_buffer = IO::Buffer.for(offsets_data)
pad!(values, buffer_padding_size(values))
values.freeze
values_buffer = IO::Buffer.for(values)
```
##########
ruby/red-arrow-format/lib/arrow-format/type.rb:
##########
@@ -742,6 +715,10 @@ def build_array(...)
def to_flatbuffers
FB::Binary::Data.new
end
+
+ def offset_pack_template
+ "l"
+ end
Review Comment:
Could you move this just after `def offset_buffer_type`?
I want to make related code close for easy to understand.
--
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]
