n3world commented on a change in pull request #10321:
URL: https://github.com/apache/arrow/pull/10321#discussion_r640039477
##########
File path: cpp/src/arrow/csv/parser.h
##########
@@ -63,19 +63,24 @@ class ARROW_EXPORT DataBatch {
uint32_t num_bytes() const { return parsed_size_; }
template <typename Visitor>
- Status VisitColumn(int32_t col_index, Visitor&& visit) const {
+ Status VisitColumn(int32_t col_index, int64_t first_row, Visitor&& visit)
const {
using detail::ParsedValueDesc;
+ int64_t row = first_row;
for (size_t buf_index = 0; buf_index < values_buffers_.size();
++buf_index) {
const auto& values_buffer = values_buffers_[buf_index];
const auto values = reinterpret_cast<const
ParsedValueDesc*>(values_buffer->data());
const auto max_pos =
static_cast<int32_t>(values_buffer->size() /
sizeof(ParsedValueDesc)) - 1;
- for (int32_t pos = col_index; pos < max_pos; pos += num_cols_) {
+ for (int32_t pos = col_index; pos < max_pos; pos += num_cols_, ++row) {
auto start = values[pos].offset;
auto stop = values[pos + 1].offset;
auto quoted = values[pos + 1].quoted;
- ARROW_RETURN_NOT_OK(visit(parsed_ + start, stop - start, quoted));
+ Status status = visit(parsed_ + start, stop - start, quoted);
+ if (ARROW_PREDICT_FALSE(first_row >= 0 && !status.ok())) {
Review comment:
I updated it to be almost what you suggested but I kept the
ARROW_RETURN_NOT_OK around status. If there is a desire to add an
ARROW_RETURN_WITh_CONTEXT macro I am happy to add that and modify the other
macros to use it. It only gets rid of a duplicate status.ok() check which isn't
much overhead so probably not really worth it.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
