pitrou commented on code in PR #45272:
URL: https://github.com/apache/arrow/pull/45272#discussion_r2179335918
##########
cpp/src/arrow/compute/kernels/scalar_compare.cc:
##########
@@ -445,6 +445,14 @@ std::shared_ptr<ScalarFunction>
MakeCompareFunction(std::string name, FunctionDo
DCHECK_OK(func->AddKernel({ty, ty}, boolean(), std::move(exec)));
}
+ if constexpr (std::is_same_v<Op, Equal> || std::is_same_v<Op, NotEqual>) {
+ for (const auto id : {Type::LIST, Type::LARGE_LIST}) {
+ auto exec = GenerateList<applicator::ScalarBinaryEqualTypes,
BooleanType, Op>(id);
Review Comment:
I see these comments at the beginning of the class:
```c++
class RangeDataEqualsImpl {
public:
// PRE-CONDITIONS:
// - the types are equal
// - the ranges are in bounds
```
We should probably add that the two ranges must have the same length.
For example this is how list comparison is currently implemented, where it
is first checked that each pair of list elements have the same length:
```c++
template <typename offset_type, typename CompareRanges>
void CompareWithOffsets(int offsets_buffer_index, CompareRanges&&
compare_ranges) {
const offset_type* left_offsets =
left_.GetValues<offset_type>(offsets_buffer_index) + left_start_idx_;
const offset_type* right_offsets =
right_.GetValues<offset_type>(offsets_buffer_index) +
right_start_idx_;
const auto compare_runs = [&](int64_t i, int64_t length) {
for (int64_t j = i; j < i + length; ++j) {
if (left_offsets[j + 1] - left_offsets[j] !=
right_offsets[j + 1] - right_offsets[j]) {
return false;
}
}
if (!compare_ranges(left_offsets[i], right_offsets[i],
left_offsets[i + length] - left_offsets[i])) {
return false;
}
return true;
};
VisitValidRuns(compare_runs);
}
```
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
