pitrou commented on a change in pull request #11197:
URL: https://github.com/apache/arrow/pull/11197#discussion_r713021577
##########
File path: cpp/examples/arrow/row_wise_conversion_example.cc
##########
@@ -129,37 +135,37 @@ arrow::Status ColumnarTableToVector(const
std::shared_ptr<arrow::Table>& table,
}
// As we have ensured that the table has the expected structure, we can
unpack the
- // underlying arrays. For the primitive columns `id` and `cost` we can use
the high
- // level functions to get the values whereas for the nested column
- // `cost_components` we need to access the C-pointer to the data to copy its
- // contents into the resulting `std::vector<double>`. Here we need to be
care to
+ // underlying arrays. For the primitive columns `id` and `components` we can
use the
+ // high level functions to get the values whereas for the nested column
+ // `component_costs` we need to access the C-pointer to the data to copy its
+ // contents into the resulting `std::vector<double>`. Here we need to be
careful to
// also add the offset to the pointer. This offset is needed to enable
zero-copy
// slicing operations. While this could be adjusted automatically for double
// arrays, this cannot be done for the accompanying bitmap as often the
slicing
// border would be inside a byte.
auto ids =
std::static_pointer_cast<arrow::Int64Array>(table->column(0)->chunk(0));
- auto costs =
- std::static_pointer_cast<arrow::DoubleArray>(table->column(1)->chunk(0));
- auto cost_components =
+ auto components =
+ std::static_pointer_cast<arrow::Int64Array>(table->column(1)->chunk(0));
+ auto component_cost =
std::static_pointer_cast<arrow::ListArray>(table->column(2)->chunk(0));
- auto cost_components_values =
- std::static_pointer_cast<arrow::DoubleArray>(cost_components->values());
+ auto component_cost_values =
+ std::static_pointer_cast<arrow::DoubleArray>(component_cost->values());
// To enable zero-copy slices, the native values pointer might need to
account
// for this slicing offset. This is not needed for the higher level functions
// like Value(…) that already account for this offset internally.
- const double* ccv_ptr = cost_components_values->data()->GetValues<double>(1);
+ const double* ccv_ptr = component_cost_values->data()->GetValues<double>(1);
Review comment:
Note this can simply be:
```c++
const double* ccv_ptr = component_cost_values->raw_values();
```
I'm not sure showcasing the buffer lookup inside ArrayData is useful here or
merely confusing?
##########
File path: cpp/examples/arrow/row_wise_conversion_example.cc
##########
@@ -47,10 +51,10 @@ struct data_row {
// construction of the final `arrow::Array` instances.
//
// For each type, Arrow has a specially typed builder class. For the primitive
-// values `id` and `cost` we can use the respective `arrow::Int64Builder` and
-// `arrow::DoubleBuilder`. For the `cost_components` vector, we need to have
two
-// builders, a top-level `arrow::ListBuilder` that builds the array of offsets
and
-// a nested `arrow::DoubleBuilder` that constructs the underlying values array
that
+// values `id` and `components` we can use the `arrow::Int64Builder`. For the
+// `component_cost` vector, we need to have two builders, a top-level
+// `arrow::ListBuilder` that builds the array of offsets and a nested
+// `arrow::DoubleBuilder` that constructs the underlying values array that
// is referenced by the offsets in the former array.
arrow::Status VectorToColumnarTable(const std::vector<struct data_row>& rows,
std::shared_ptr<arrow::Table>* table) {
Review comment:
Since we are updating this function, can it perhaps return
`Result<std::shared_ptr<Table>>` instead of `Status`?
See https://arrow.apache.org/docs/cpp/conventions.html#error-reporting
--
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]
