bkietz commented on a change in pull request #10397:
URL: https://github.com/apache/arrow/pull/10397#discussion_r660110016
##########
File path: cpp/src/arrow/util/future.h
##########
@@ -976,4 +979,43 @@ Future<BreakValueType> Loop(Iterate iterate) {
return break_fut;
}
+template <typename T>
+struct EnsureFuture {
+ using type = Future<T>;
+};
+
+template <typename T>
+struct EnsureFuture<Result<T>> {
+ using type = Future<T>;
+};
+
+template <typename T>
+struct EnsureFuture<Future<T>> {
+ using type = Future<T>;
+};
+
+template <>
+struct EnsureFuture<Status> {
+ using type = Future<>;
+};
Review comment:
Nice
##########
File path: cpp/src/arrow/util/thread_pool_test.cc
##########
@@ -395,6 +395,22 @@ TEST_F(TestThreadPool, StressSpawn) {
SpawnAdds(pool.get(), 1000, task_add<int>);
}
+TEST_F(TestThreadPool, OwnsCurrentThread) {
+ auto pool = this->MakeThreadPool(30);
+ std::atomic<bool> one_failed{false};
+
+ for (int i = 0; i < 1000; ++i) {
+ ASSERT_OK(pool->Spawn([&] {
+ if (pool->OwnsThisThread()) return;
+
+ one_failed = true;
+ }));
+ }
+
+ ASSERT_OK(pool->Shutdown());
+ ASSERT_FALSE(one_failed);
Review comment:
will do
##########
File path: cpp/src/arrow/testing/gtest_util.h
##########
@@ -28,6 +28,7 @@
#include <utility>
#include <vector>
+#include <gmock/gmock-matchers.h>
Review comment:
I'll move these to `arrow/testing/matchers.h`, then
##########
File path: cpp/src/arrow/compute/exec/plan_test.cc
##########
@@ -20,27 +20,46 @@
#include <functional>
#include <memory>
+#include "arrow/compute/exec.h"
#include "arrow/compute/exec/exec_plan.h"
+#include "arrow/compute/exec/expression.h"
#include "arrow/compute/exec/test_util.h"
#include "arrow/record_batch.h"
#include "arrow/testing/future_util.h"
#include "arrow/testing/gtest_util.h"
#include "arrow/testing/random.h"
#include "arrow/util/logging.h"
#include "arrow/util/thread_pool.h"
+#include "arrow/util/vector.h"
-namespace arrow {
+using testing::ElementsAre;
+using testing::HasSubstr;
+using testing::UnorderedElementsAreArray;
-using internal::Executor;
+namespace arrow {
namespace compute {
-void AssertBatchesEqual(const RecordBatchVector& expected,
- const RecordBatchVector& actual) {
- ASSERT_EQ(expected.size(), actual.size());
- for (size_t i = 0; i < expected.size(); ++i) {
- AssertBatchesEqual(*expected[i], *actual[i]);
+ExecBatch ExecBatchFromJSON(const std::vector<ValueDescr>& descrs,
Review comment:
I'll put it in arrow/compute/test_util.h
##########
File path: cpp/src/arrow/compute/exec/plan_test.cc
##########
@@ -20,27 +20,46 @@
#include <functional>
#include <memory>
+#include "arrow/compute/exec.h"
#include "arrow/compute/exec/exec_plan.h"
+#include "arrow/compute/exec/expression.h"
#include "arrow/compute/exec/test_util.h"
#include "arrow/record_batch.h"
#include "arrow/testing/future_util.h"
#include "arrow/testing/gtest_util.h"
#include "arrow/testing/random.h"
#include "arrow/util/logging.h"
#include "arrow/util/thread_pool.h"
+#include "arrow/util/vector.h"
-namespace arrow {
+using testing::ElementsAre;
+using testing::HasSubstr;
+using testing::UnorderedElementsAreArray;
-using internal::Executor;
+namespace arrow {
namespace compute {
-void AssertBatchesEqual(const RecordBatchVector& expected,
- const RecordBatchVector& actual) {
- ASSERT_EQ(expected.size(), actual.size());
- for (size_t i = 0; i < expected.size(); ++i) {
- AssertBatchesEqual(*expected[i], *actual[i]);
+ExecBatch ExecBatchFromJSON(const std::vector<ValueDescr>& descrs,
Review comment:
I'll put it in arrow/compute/exec/test_util.h
##########
File path: cpp/src/arrow/compute/exec/expression.cc
##########
@@ -61,13 +61,22 @@ Expression call(std::string function,
std::vector<Expression> arguments,
call.function_name = std::move(function);
call.arguments = std::move(arguments);
call.options = std::move(options);
+
+ call.hash = std::hash<std::string>{}(call.function_name);
+ for (const auto& arg : call.arguments) {
+ call.hash ^= arg.hash();
Review comment:
thanks, will replace
##########
File path: cpp/src/arrow/util/vector.h
##########
@@ -84,27 +84,49 @@ std::vector<T> FilterVector(std::vector<T> values,
Predicate&& predicate) {
return values;
}
-/// \brief Like MapVector, but where the function can fail.
-template <typename Fn, typename From = internal::call_traits::argument_type<0,
Fn>,
- typename To = typename
internal::call_traits::return_type<Fn>::ValueType>
-Result<std::vector<To>> MaybeMapVector(Fn&& map, const std::vector<From>& src)
{
+template <typename Fn, typename From,
+ typename To = decltype(std::declval<Fn>()(std::declval<From>()))>
+std::vector<To> MapVector(Fn&& map, const std::vector<From>& source) {
std::vector<To> out;
- out.reserve(src.size());
- ARROW_RETURN_NOT_OK(MaybeTransform(src.begin(), src.end(),
std::back_inserter(out),
- std::forward<Fn>(map)));
- return std::move(out);
+ out.reserve(source.size());
+ std::transform(source.begin(), source.end(), std::back_inserter(out),
+ std::forward<Fn>(map));
+ return out;
}
template <typename Fn, typename From,
typename To = decltype(std::declval<Fn>()(std::declval<From>()))>
-std::vector<To> MapVector(Fn&& map, const std::vector<From>& source) {
+std::vector<To> MapVector(Fn&& map, std::vector<From>&& source) {
std::vector<To> out;
out.reserve(source.size());
- std::transform(source.begin(), source.end(), std::back_inserter(out),
+ std::transform(std::make_move_iterator(source.begin()),
+ std::make_move_iterator(source.end()),
std::back_inserter(out),
std::forward<Fn>(map));
return out;
}
+/// \brief Like MapVector, but where the function can fail.
+template <typename Fn, typename From = internal::call_traits::argument_type<0,
Fn>,
+ typename To = typename
internal::call_traits::return_type<Fn>::ValueType>
Review comment:
there's not a good reason; just uniformity with getting `From` from
call_traits.
##########
File path: cpp/src/arrow/type.cc
##########
@@ -1195,6 +1195,10 @@ std::string FieldRef::ToString() const {
}
std::vector<FieldPath> FieldRef::FindAll(const Schema& schema) const {
+ if (auto name = this->name()) {
+ return internal::MapVector([](int i) { return FieldPath{i}; },
+ schema.GetAllFieldIndices(*name));
+ }
Review comment:
This is tested
https://github.com/bkietz/arrow/blob/1b8bedcbf4b794858228c85644248300b64ce5a4/cpp/src/arrow/type_test.cc#L391-L392
--
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]
