[GitHub] [arrow] drin commented on a diff in pull request #13518: ARROW-16700: [C++][R][Datasets] aggregates on partitioning columns

[GitBox]

drin commented on code in PR #13518:
URL: https://github.com/apache/arrow/pull/13518#discussion_r922566935


##########
cpp/src/arrow/dataset/scanner_test.cc:
##########
@@ -1445,9 +1446,13 @@ DatasetAndBatches MakeBasicDataset() {
           equal(field_ref("c"), literal(23)),
           equal(field_ref("c"), literal(47)),
       },
-      [](compute::ExecBatch* batch, const RecordBatch&) {
+      [](compute::ExecBatch* batch, const RecordBatch&, size_t 
guarantee_index) {
         // a placeholder will be inserted for partition field "c"
-        batch->values.emplace_back(std::make_shared<Int32Scalar>());
+        if (guarantee_index == 0) {
+          batch->values.emplace_back(std::make_shared<Int32Scalar>(23));
+        } else {
+          batch->values.emplace_back(std::make_shared<Int32Scalar>(47));
+        }

Review Comment:
   I address this in 
https://github.com/apache/arrow/pull/13518/commits/8b9137e8fb7ca0d4d017bc995d6705a271ae4141.
   
   I essentially change the invocation of the lambda to:
   ```cpp
   if (not guarantees.empty()) {
     auto extract_result = ExtractKnownFieldValues(guarantees[frag_ndx]);
     ARROW_WARN_NOT_OK(extract_result.status(), "ExtractKnownFieldValues 
failed");
     for (const auto& known_field : extract_result->map) {
       batches.back().values.emplace_back(known_field.second);
     }
   }
   ```
   
   ooh, but I just realized I used `not` instead of `!`. I'm not sure if 
there's a preference



-- 
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