Fix invalid use of std::exclusive_scan in Parquet writer #13434
Conversation
|
Pull requests from external contributors require approval from a |
|
/ok to test |
cpp/src/io/parquet/writer_impl.cu
Outdated
| @@ -1710,10 +1710,10 @@ auto convert_table_to_parquet_data(table_input_metadata& table_meta, | |||
| size_type const total_frags = [&]() { | |||
| if (frags_per_column.size() > 0) { | |||
| std::exclusive_scan(frags_per_column.data(), | |||
| frags_per_column.data() + num_columns + 1, | |||
| frags_per_column.data() + num_columns, | |||
There was a problem hiding this comment.
Let's use frags_per_column.begin(), frags_per_column.end().
There was a problem hiding this comment.
Also please avoid back_inserter if possible. So this should be better:
std::vector<size_type> frag_offsets(num_columns, 0);
...
std::exclusive_scan(frags_per_column.begin(), frags_per_column.end(), frag_offsets.begin(), 0);
There was a problem hiding this comment.
much cleaner. thanks!
There was a problem hiding this comment.
AFAIK, the canonical way is to reserve the final size and use back_inserter, so we never reallocate and never default-initialize elements. It is unfortunatelly also the most verbose option 🤷♂️
Not a request to apply this here, just an FYI (or maybe a FMI, if @ttnghia knows of a back_inserter drawback).
There was a problem hiding this comment.
I like that. I wouldn't mind changing to that just so it's in my muscle memory :)
Co-authored-by: Nghia Truong <[email protected]>
|
/ok to test |
|
/ok to test |
|
/merge |
Description
Fixes #13431
Checklist