bkietz commented on a change in pull request #9095:
URL: https://github.com/apache/arrow/pull/9095#discussion_r562883545
##########
File path: cpp/src/arrow/csv/reader.cc
##########
@@ -150,20 +154,21 @@ struct CSVBlock {
std::function<Status(int64_t)> consume_bytes;
};
+// This is an unfortunate side-effect of using optional<T> as the iterator in
the
+// CSVBlock iterator. We need to be able to compare with
+// IterationTraits<optional<T>>::End() and empty optionals will always compare
true but
+// the optional copmarator won't compile if the underlying type isn't
comparable
+bool operator==(const CSVBlock& left, const CSVBlock& right) { return false; }
Review comment:
Could we just have
```suggestion
bool operator==(const CSVBlock& left, const CSVBlock& right) { return
left.block_index == right.block_index; }
```
? It'd still be worthwhile to explain that equality comparability is part of
the contract of `Iterator`.
##########
File path: cpp/src/arrow/csv/reader.cc
##########
@@ -150,20 +154,21 @@ struct CSVBlock {
std::function<Status(int64_t)> consume_bytes;
};
+// This is an unfortunate side-effect of using optional<T> as the iterator in
the
+// CSVBlock iterator. We need to be able to compare with
+// IterationTraits<optional<T>>::End() and empty optionals will always compare
true but
+// the optional copmarator won't compile if the underlying type isn't
comparable
+bool operator==(const CSVBlock& left, const CSVBlock& right) { return false; }
Review comment:
You could additionally skip `optional` entirely if desired by defining
`IterationTraits<CSVBlock>::End()` to have block_index == -1 or so.
----------------------------------------------------------------
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:
[email protected]
