AlenkaF commented on code in PR #40359:
URL: https://github.com/apache/arrow/pull/40359#discussion_r1514408190
##########
cpp/src/arrow/record_batch.cc:
##########
@@ -269,28 +320,46 @@ Result<std::shared_ptr<Tensor>>
RecordBatch::ToTensor(MemoryPool* pool) const {
"Conversion to Tensor for RecordBatches without columns/schema is not "
"supported.");
}
- const auto& type = column(0)->type();
- // Check for supported data types
- if (!is_integer(type->id()) && !is_floating(type->id())) {
- return Status::TypeError("DataType is not supported: ", type->ToString());
- }
- // Check for uniform data type
// Check for no validity bitmap of each field
for (int i = 0; i < num_columns(); ++i) {
if (column(i)->null_count() > 0) {
return Status::TypeError("Can only convert a RecordBatch with no
nulls.");
}
- if (column(i)->type() != type) {
- return Status::TypeError("Can only convert a RecordBatch with uniform
data type.");
+ }
+
+ // Check for supported data types and merge fields
+ // to get the resulting uniform data type
+ if (!is_integer(column(0)->type()->id()) &&
!is_floating(column(0)->type()->id())) {
+ return Status::TypeError("DataType is not supported: ",
+ column(0)->type()->ToString());
+ }
+ std::shared_ptr<Field> result_field = schema_->field(0);
+ std::shared_ptr<DataType> result_type = result_field->type();
+
+ if (num_columns() > 1) {
+ Field::MergeOptions options;
+ options.promote_integer_to_float = true;
+ options.promote_integer_sign = true;
+ options.promote_numeric_width = true;
+
+ for (int i = 1; i < num_columns(); ++i) {
+ if (!is_integer(column(i)->type()->id()) &&
!is_floating(column(i)->type()->id())) {
+ return Status::TypeError("DataType is not supported: ",
+ column(i)->type()->ToString());
+ }
+ ARROW_ASSIGN_OR_RAISE(
+ result_field, result_field->MergeWith(
Review Comment:
It is not very obvious to me how to export `MergeTypes` as it is a utility
for `Field::MergeWith` and is dependent on other utility functions. I think the
only way would be to move all the utility functions for `Field::MergeWith` into
the header file. If that is correct, the current solutions seems better to me,
even though using `MergeTypes` in `RecordBatch::ToTensor` would be nicer.
--
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]
