You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/01/12 17:46:41 UTC

[GitHub] [arrow] bkietz commented on a change in pull request #9164: ARROW-10663: [C++] Fix is_in and index_in behaviour

bkietz commented on a change in pull request #9164:
URL: https://github.com/apache/arrow/pull/9164#discussion_r555954700



##########
File path: cpp/src/arrow/compute/kernels/scalar_set_lookup.cc
##########
@@ -36,53 +36,59 @@ namespace {
 
 template <typename Type>
 struct SetLookupState : public KernelState {
-  explicit SetLookupState(MemoryPool* pool)
-      : lookup_table(pool, 0), lookup_null_count(0) {}
+  explicit SetLookupState(const SetLookupOptions& options, MemoryPool* pool)
+      : options(options), lookup_table(pool, 0) {}
 
-  Status Init(const SetLookupOptions& options) {
+  Status Init() {
+    if (options.value_set.kind() == Datum::ARRAY) {
+      RETURN_NOT_OK(AddArrayValueSet(*options.value_set.array()));
+    } else if (options.value_set.kind() == Datum::CHUNKED_ARRAY) {
+      const ChunkedArray& value_set = *options.value_set.chunked_array();
+      for (const std::shared_ptr<Array>& chunk : value_set.chunks()) {

Review comment:
       We have a lot of code like this, maybe later we should add `Datum::chunks()` so we can write
   ```c++
   if (!options.values_set.is_arraylike()) {
     return Status::Invalid("value_set should be an array or chunked array");
   }
   for (const std::shared_ptr<ArrayData>& chunk : options.value_set.chunks()) {
     RETURN_NOT_OK(AddArrayValueSet(*chunk->data()));
   }
   ```




----------------------------------------------------------------
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:
users@infra.apache.org