kou commented on code in PR #51214:
URL: https://github.com/apache/arrow/pull/51214#discussion_r3953908079
##########
ruby/red-arrow-format/lib/arrow-format/record-batch.rb:
##########
@@ -143,6 +148,52 @@ def all_buffers_enumerator
end
private
+ def build_with_schema(schema, data)
+ fields = schema.fields
+ name_to_index = {}
+ fields.each_with_index do |field, i|
+ name_to_index[field.name] = i
+ end
+ if data.is_a?(Hash)
+ raw_columns = fields.collect { [] }
+ data.each do |name, values|
+ raw_columns[name_to_index[name.to_s]] = values
Review Comment:
Should we ignore nonexistent `name` case?
##########
ruby/red-arrow-format/lib/arrow-format/record-batch.rb:
##########
@@ -143,6 +148,52 @@ def all_buffers_enumerator
end
private
+ def build_with_schema(schema, data)
+ fields = schema.fields
+ name_to_index = {}
+ fields.each_with_index do |field, i|
+ name_to_index[field.name] = i
+ end
+ if data.is_a?(Hash)
+ raw_columns = fields.collect { [] }
Review Comment:
It seems that we don't need to create empty arrays: `raw_columns = []`
##########
ruby/red-arrow-format/lib/arrow-format/record-batch.rb:
##########
@@ -143,6 +148,52 @@ def all_buffers_enumerator
end
private
+ def build_with_schema(schema, data)
+ fields = schema.fields
+ name_to_index = {}
+ fields.each_with_index do |field, i|
+ name_to_index[field.name] = i
+ end
+ if data.is_a?(Hash)
+ raw_columns = fields.collect { [] }
+ data.each do |name, values|
+ raw_columns[name_to_index[name.to_s]] = values
+ end
+ columns = fields.zip(raw_columns).collect do |field, values|
+ field.type.build_array(values)
+ end
+ else
+ raw_columns = fields.collect { [] }
+ data.each_with_index do |record, nth_record|
+ case record
+ when nil
+ when Hash
+ record.each do |name, value|
+ field_index = name_to_index[name.to_s]
+ raw_columns[field_index] << value if field_index
+ end
+ else
+ record.each_with_index do |value, field_index|
+ raw_columns[field_index] << value
+ end
+ end
+ raw_columns.each do |column|
+ column << nil if column.size != nth_record + 1
+ end
+ end
+ columns = fields.zip(raw_columns).collect do |field, values|
+ field.type.build_array(values)
+ end
+ end
+ n_rows = columns.first&.size || 0
+ all_n_rows = columns.collect(&:size)
+ if all_n_rows.uniq.size != 1
+ message = "inconsistent the number of rows: #{all_n_rows.join(", ")}"
+ raise ArgumentError, message
+ end
Review Comment:
It seems that this check is needed only for `if data.is_a?(Hash)`.
##########
ruby/red-arrow-format/lib/arrow-format/record-batch.rb:
##########
@@ -143,6 +148,52 @@ def all_buffers_enumerator
end
private
+ def build_with_schema(schema, data)
+ fields = schema.fields
+ name_to_index = {}
+ fields.each_with_index do |field, i|
+ name_to_index[field.name] = i
+ end
+ if data.is_a?(Hash)
+ raw_columns = fields.collect { [] }
+ data.each do |name, values|
+ raw_columns[name_to_index[name.to_s]] = values
+ end
+ columns = fields.zip(raw_columns).collect do |field, values|
+ field.type.build_array(values)
+ end
+ else
+ raw_columns = fields.collect { [] }
+ data.each_with_index do |record, nth_record|
+ case record
+ when nil
+ when Hash
+ record.each do |name, value|
+ field_index = name_to_index[name.to_s]
+ raw_columns[field_index] << value if field_index
+ end
+ else
+ record.each_with_index do |value, field_index|
+ raw_columns[field_index] << value
Review Comment:
Should we ignore out-of-range `field_index` or raise an exception for the
case?
--
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]
