lidavidm commented on a change in pull request #11019:
URL: https://github.com/apache/arrow/pull/11019#discussion_r706231044
##########
File path: cpp/src/arrow/compute/api_vector.cc
##########
@@ -140,6 +143,29 @@ PartitionNthOptions::PartitionNthOptions(int64_t pivot)
: FunctionOptions(internal::kPartitionNthOptionsType), pivot(pivot) {}
constexpr char PartitionNthOptions::kTypeName[];
+SelectKOptions::SelectKOptions(int64_t k, std::vector<SortKey> sort_keys)
+ : FunctionOptions(internal::kSelectKOptionsType),
+ k(k),
+ sort_keys(std::move(sort_keys)) {}
+
+bool SelectKOptions::is_top_k() const {
+ for (const auto& k : sort_keys) {
+ if (k.order != SortOrder::Descending) {
+ return false;
+ }
Review comment:
They only matter for the array/chunked array cases. In that case I'd
almost rather inline the definition and delete `SelectKOptions::is_top_k` since
otherwise it would be rather confusing as a public API, i.e. instead of
`select_k_options.is_top_k()` have 1) a check that there are sort keys and then
2) check `select_k_options.sort_keys[0].order == Descending` instead, inside
the function definition.
--
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]
