raulcd commented on code in PR #43698:
URL: https://github.com/apache/arrow/pull/43698#discussion_r1916237498


##########
cpp/src/arrow/util/thread_pool.h:
##########
@@ -602,11 +604,16 @@ typename Fut::SyncType 
RunSynchronously(FnOnce<Fut(Executor*)> get_future,
 ///   the iterator will use the calling thread to do enough work to generate 
one item.
 ///   Tasks will be left in a queue until the next call and no work will be 
done between
 ///   calls.
+///
+/// If `executor` is null, then the default CPU thread pool will be used.
+/// If `executor` is not null, then it will be used.
 template <typename T>
 Iterator<T> IterateSynchronously(
-    FnOnce<Result<std::function<Future<T>()>>(Executor*)> get_gen, bool 
use_threads) {
+    FnOnce<Result<std::function<Future<T>()>>(Executor*)> get_gen, bool 
use_threads,
+    Executor* executor) {
   if (use_threads) {
-    auto maybe_gen = std::move(get_gen)(GetCpuThreadPool());
+    auto used_executor = executor != nullptr ? executor : GetCpuThreadPool();

Review Comment:
   We seem to require our `NULLPTR` macro usage on header files due to C++/CLI 
compatibilty. This is why is failing linting.
   To be honest I am not sure whether the C++/CLI compatibiltiy is required 
anymore so I am asking on Zulip and I might follow up in the ML.
   ```suggestion
       auto used_executor = executor != NULLPTR ? executor : GetCpuThreadPool();
   ```



##########
cpp/src/arrow/util/thread_pool.h:
##########
@@ -616,5 +623,23 @@ Iterator<T> IterateSynchronously(
   }
 }
 
+/// \brief Potentially iterate an async generator serially (if use_threads is 
false)
+///   using the default CPU thread pool
+/// \see IterateGenerator
+///
+/// If `use_threads` is true, the global CPU executor will be used.  Each call 
to
+///   the iterator will simply wait until the next item is available.  Tasks 
may run in
+///   the background between calls.
+///
+/// If `use_threads` is false, the calling thread only will be used.  Each 
call to
+///   the iterator will use the calling thread to do enough work to generate 
one item.
+///   Tasks will be left in a queue until the next call and no work will be 
done between
+///   calls.
+template <typename T>
+Iterator<T> IterateSynchronously(
+    FnOnce<Result<std::function<Future<T>()>>(Executor*)> get_gen, bool 
use_threads) {
+  return IterateSynchronously(std::move(get_gen), use_threads, nullptr);

Review Comment:
   ```suggestion
     return IterateSynchronously(std::move(get_gen), use_threads, NULLPTR);
   ```



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

Reply via email to