diff --git a/c_glib/test/test-decimal128.rb b/c_glib/test/test-decimal128.rb index d032afd510d..c9405326fae 100644 --- a/c_glib/test/test-decimal128.rb +++ b/c_glib/test/test-decimal128.rb @@ -123,7 +123,7 @@ def test_divide_zero decimal1 = Arrow::Decimal128.new(23423445) decimal2 = Arrow::Decimal128.new(0) message = - "[decimal128][divide]: Invalid: Division by 0 in Decimal128" + "[decimal128][divide]: Invalid: Division by 0 in Decimal" assert_raise(Arrow::Error::Invalid.new(message)) do decimal1.divide(decimal2) end @@ -236,7 +236,7 @@ def test_rescale_fail decimal = Arrow::Decimal128.new(10) message = "[decimal128][rescale]: Invalid: " + - "Rescaling Decimal128 value would cause data loss" + "Rescaling Decimal value would cause data loss" assert_raise(Arrow::Error::Invalid.new(message)) do decimal.rescale(1, -1) end diff --git a/c_glib/test/test-decimal256.rb b/c_glib/test/test-decimal256.rb index 24fd3b5552b..0592972286b 100644 --- a/c_glib/test/test-decimal256.rb +++ b/c_glib/test/test-decimal256.rb @@ -110,7 +110,7 @@ def test_divide_zero decimal1 = Arrow::Decimal256.new(23423445) decimal2 = Arrow::Decimal256.new(0) message = - "[decimal256][divide]: Invalid: Division by 0 in Decimal256" + "[decimal256][divide]: Invalid: Division by 0 in Decimal" assert_raise(Arrow::Error::Invalid.new(message)) do decimal1.divide(decimal2) end @@ -223,7 +223,7 @@ def test_rescale_fail decimal = Arrow::Decimal256.new(10) message = "[decimal256][rescale]: Invalid: " + - "Rescaling Decimal256 value would cause data loss" + "Rescaling Decimal value would cause data loss" assert_raise(Arrow::Error::Invalid.new(message)) do decimal.rescale(1, -1) end diff --git a/c_glib/test/test-decimal32.rb b/c_glib/test/test-decimal32.rb index 33b84ccc6b5..83b719251f7 100644 --- a/c_glib/test/test-decimal32.rb +++ b/c_glib/test/test-decimal32.rb @@ -106,7 +106,7 @@ def test_divide_zero decimal1 = Arrow::Decimal32.new(23423445) decimal2 = Arrow::Decimal32.new(0) message = - "[decimal32][divide]: Invalid: Division by 0 in Decimal32" + "[decimal32][divide]: Invalid: Division by 0 in Decimal" assert_raise(Arrow::Error::Invalid.new(message)) do decimal1.divide(decimal2) end @@ -214,7 +214,7 @@ def test_rescale_fail decimal = Arrow::Decimal32.new(10) message = "[decimal32][rescale]: Invalid: " + - "Rescaling Decimal32 value would cause data loss" + "Rescaling Decimal value would cause data loss" assert_raise(Arrow::Error::Invalid.new(message)) do decimal.rescale(1, -1) end diff --git a/c_glib/test/test-decimal64.rb b/c_glib/test/test-decimal64.rb index add4f3e0b49..3fd7f3b4198 100644 --- a/c_glib/test/test-decimal64.rb +++ b/c_glib/test/test-decimal64.rb @@ -106,7 +106,7 @@ def test_divide_zero decimal1 = Arrow::Decimal64.new(23423445) decimal2 = Arrow::Decimal64.new(0) message = - "[decimal64][divide]: Invalid: Division by 0 in Decimal64" + "[decimal64][divide]: Invalid: Division by 0 in Decimal" assert_raise(Arrow::Error::Invalid.new(message)) do decimal1.divide(decimal2) end @@ -214,7 +214,7 @@ def test_rescale_fail decimal = Arrow::Decimal64.new(10) message = "[decimal64][rescale]: Invalid: " + - "Rescaling Decimal64 value would cause data loss" + "Rescaling Decimal value would cause data loss" assert_raise(Arrow::Error::Invalid.new(message)) do decimal.rescale(1, -1) end diff --git a/cpp/src/arrow/array/concatenate_test.cc b/cpp/src/arrow/array/concatenate_test.cc index aea53115752..c09a6b45a70 100644 --- a/cpp/src/arrow/array/concatenate_test.cc +++ b/cpp/src/arrow/array/concatenate_test.cc @@ -430,14 +430,14 @@ TEST_F(ConcatenateTest, DictionaryTypeDifferentSizeIndex) { auto bigger_dict_type = dictionary(uint16(), utf8()); auto dict_one = DictArrayFromJSON(dict_type, "[0]", "[\"A0\"]"); auto dict_two = DictArrayFromJSON(bigger_dict_type, "[0]", "[\"B0\"]"); - ASSERT_RAISES(Invalid, Concatenate({dict_one, dict_two}).status()); + ASSERT_RAISES(Invalid, Concatenate({dict_one, dict_two})); } TEST_F(ConcatenateTest, DictionaryTypeCantUnifyNullInDictionary) { auto dict_type = dictionary(uint8(), utf8()); auto dict_one = DictArrayFromJSON(dict_type, "[0, 1]", "[null, \"A\"]"); auto dict_two = DictArrayFromJSON(dict_type, "[0, 1]", "[null, \"B\"]"); - ASSERT_RAISES(Invalid, Concatenate({dict_one, dict_two}).status()); + ASSERT_RAISES(Invalid, Concatenate({dict_one, dict_two})); } TEST_F(ConcatenateTest, DictionaryTypeEnlargedIndices) { @@ -464,7 +464,7 @@ TEST_F(ConcatenateTest, DictionaryTypeEnlargedIndices) { auto dict_one = std::make_shared(dict_type, indices, dictionary_one); auto dict_two = std::make_shared(dict_type, indices, dictionary_two); - ASSERT_RAISES(Invalid, Concatenate({dict_one, dict_two}).status()); + ASSERT_RAISES(Invalid, Concatenate({dict_one, dict_two})); auto bigger_dict_type = dictionary(uint16(), uint16()); @@ -729,8 +729,7 @@ TEST_F(ConcatenateTest, OffsetOverflow) { fake_long_list->data()->child_data[0] = fake_long->data(); ASSERT_RAISES(Invalid, internal::Concatenate({fake_long_list, fake_long_list}, pool, - &suggested_cast) - .status()); + &suggested_cast)); ASSERT_TRUE(suggested_cast->Equals(*expected_suggestion)); } } @@ -740,8 +739,7 @@ TEST_F(ConcatenateTest, OffsetOverflow) { fake_long_list->data()->GetMutableValues(1)[1] = std::numeric_limits::max(); ASSERT_RAISES(Invalid, internal::Concatenate({fake_long_list, fake_long_list}, pool, - &suggested_cast) - .status()); + &suggested_cast)); ASSERT_TRUE(suggested_cast->Equals(LargeVersionOfType(list_ty))); auto list_view_ty = list_view(null()); @@ -757,8 +755,7 @@ TEST_F(ConcatenateTest, OffsetOverflow) { mutable_sizes[0] = kInt32Max; } ASSERT_RAISES(Invalid, internal::Concatenate({fake_long_list_view, fake_long_list_view}, - pool, &suggested_cast) - .status()); + pool, &suggested_cast)); ASSERT_TRUE(suggested_cast->Equals(LargeVersionOfType(list_view_ty))); } diff --git a/cpp/src/arrow/array/diff_test.cc b/cpp/src/arrow/array/diff_test.cc index 3effe2a0372..76f4202992f 100644 --- a/cpp/src/arrow/array/diff_test.cc +++ b/cpp/src/arrow/array/diff_test.cc @@ -76,7 +76,7 @@ class DiffTest : public ::testing::Test { void DoDiff() { auto edits = Diff(*base_, *target_, default_memory_pool()); - ASSERT_OK(edits.status()); + ASSERT_OK(edits); edits_ = edits.ValueOrDie(); ASSERT_OK(edits_->ValidateFull()); ASSERT_TRUE(edits_->type()->Equals(edits_type)); @@ -87,7 +87,7 @@ class DiffTest : public ::testing::Test { void DoDiffAndFormat(std::stringstream* out) { DoDiff(); auto formatter = MakeUnifiedDiffFormatter(*base_->type(), out); - ASSERT_OK(formatter.status()); + ASSERT_OK(formatter); ASSERT_OK(formatter.ValueOrDie()(*edits_, *base_, *target_)); } @@ -800,10 +800,10 @@ TEST_F(DiffTest, CompareRandomStruct) { auto type = struct_({field("i", int32()), field("s", utf8())}); auto base_res = StructArray::Make({int32_base, utf8_base}, type->fields()); - ASSERT_OK(base_res.status()); + ASSERT_OK(base_res); base_ = base_res.ValueOrDie(); auto target_res = StructArray::Make({int32_target, utf8_target}, type->fields()); - ASSERT_OK(target_res.status()); + ASSERT_OK(target_res); target_ = target_res.ValueOrDie(); std::stringstream formatted; diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic_benchmark.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic_benchmark.cc index 908c642bae8..20a824cd7c3 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic_benchmark.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic_benchmark.cc @@ -80,7 +80,7 @@ static void ArrayScalarKernel(benchmark::State& state) { rand.Numeric(array_size, min, max, args.null_proportion)); for (auto _ : state) { - ABORT_NOT_OK(Op(lhs, rhs, ArithmeticOptions(), nullptr).status()); + ABORT_NOT_OK(Op(lhs, rhs, ArithmeticOptions(), nullptr)); } state.SetItemsProcessed(state.iterations() * array_size); } @@ -103,7 +103,7 @@ static void ArrayArrayKernel(benchmark::State& state) { rand.Numeric(array_size, rmin, rmax, args.null_proportion)); for (auto _ : state) { - ABORT_NOT_OK(Op(lhs, rhs, ArithmeticOptions(), nullptr).status()); + ABORT_NOT_OK(Op(lhs, rhs, ArithmeticOptions(), nullptr)); } state.SetItemsProcessed(state.iterations() * array_size); } diff --git a/cpp/src/arrow/compute/kernels/scalar_boolean_benchmark.cc b/cpp/src/arrow/compute/kernels/scalar_boolean_benchmark.cc index 89091186ae2..73fc151b74b 100644 --- a/cpp/src/arrow/compute/kernels/scalar_boolean_benchmark.cc +++ b/cpp/src/arrow/compute/kernels/scalar_boolean_benchmark.cc @@ -42,7 +42,7 @@ static void ArrayArrayKernel(benchmark::State& state) { auto rhs = rand.Boolean(array_size, /*true_probability=*/0.5, args.null_proportion); for (auto _ : state) { - ABORT_NOT_OK(Op(lhs, rhs, nullptr).status()); + ABORT_NOT_OK(Op(lhs, rhs, nullptr)); } state.SetItemsProcessed(state.iterations() * array_size); } diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_benchmark.cc b/cpp/src/arrow/compute/kernels/scalar_cast_benchmark.cc index 04749a5bea2..3899254c9b9 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_benchmark.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_benchmark.cc @@ -37,7 +37,7 @@ static void BenchmarkNumericCast(benchmark::State& state, random::RandomArrayGenerator rand(kSeed); auto array = rand.Numeric(args.size, min, max, args.null_proportion); for (auto _ : state) { - ABORT_NOT_OK(Cast(array, to_type, options).status()); + ABORT_NOT_OK(Cast(array, to_type, options)); } } @@ -54,7 +54,7 @@ static void BenchmarkFloatingToIntegerCast(benchmark::State& state, std::shared_ptr values_as_float = *Cast(*array, from_type); for (auto _ : state) { - ABORT_NOT_OK(Cast(values_as_float, to_type, options).status()); + ABORT_NOT_OK(Cast(values_as_float, to_type, options)); } } diff --git a/cpp/src/arrow/compute/kernels/scalar_compare_benchmark.cc b/cpp/src/arrow/compute/kernels/scalar_compare_benchmark.cc index fdfd63498f5..6e2bfc4434d 100644 --- a/cpp/src/arrow/compute/kernels/scalar_compare_benchmark.cc +++ b/cpp/src/arrow/compute/kernels/scalar_compare_benchmark.cc @@ -38,7 +38,7 @@ static void BenchArrayScalar(benchmark::State& state, const std::string& op) { auto array = rand.ArrayOf(ty, args.size, args.null_proportion); auto scalar = *rand.ArrayOf(ty, 1, 0)->GetScalar(0); for (auto _ : state) { - ABORT_NOT_OK(CallFunction(op, {array, Datum(scalar)}).status()); + ABORT_NOT_OK(CallFunction(op, {array, Datum(scalar)})); } } @@ -50,7 +50,7 @@ static void BenchArrayArray(benchmark::State& state, const std::string& op) { auto lhs = rand.ArrayOf(ty, args.size, args.null_proportion); auto rhs = rand.ArrayOf(ty, args.size, args.null_proportion); for (auto _ : state) { - ABORT_NOT_OK(CallFunction(op, {lhs, rhs}).status()); + ABORT_NOT_OK(CallFunction(op, {lhs, rhs})); } } diff --git a/cpp/src/arrow/compute/kernels/scalar_list_benchmark.cc b/cpp/src/arrow/compute/kernels/scalar_list_benchmark.cc index cf8503d6868..4877df46a7a 100644 --- a/cpp/src/arrow/compute/kernels/scalar_list_benchmark.cc +++ b/cpp/src/arrow/compute/kernels/scalar_list_benchmark.cc @@ -41,7 +41,7 @@ static void BenchmarkListSlice(benchmark::State& state, const ListSliceOptions& auto ctx = default_exec_context(); std::vector input_args = {std::move(array)}; for (auto _ : state) { - ABORT_NOT_OK(CallFunction("list_slice", input_args, &opts, ctx).status()); + ABORT_NOT_OK(CallFunction("list_slice", input_args, &opts, ctx)); } } diff --git a/cpp/src/arrow/compute/kernels/scalar_random_benchmark.cc b/cpp/src/arrow/compute/kernels/scalar_random_benchmark.cc index bf4a339a9bd..baf698bede5 100644 --- a/cpp/src/arrow/compute/kernels/scalar_random_benchmark.cc +++ b/cpp/src/arrow/compute/kernels/scalar_random_benchmark.cc @@ -30,7 +30,7 @@ static void RandomKernel(benchmark::State& state, bool is_seed) { const auto options = is_seed ? RandomOptions::FromSeed(42) : RandomOptions::FromSystemRandom(); for (auto _ : state) { - ABORT_NOT_OK(CallFunction("random", ExecBatch({}, length), &options).status()); + ABORT_NOT_OK(CallFunction("random", ExecBatch({}, length), &options)); } state.SetItemsProcessed(state.iterations() * length); } diff --git a/cpp/src/arrow/compute/kernels/scalar_temporal_benchmark.cc b/cpp/src/arrow/compute/kernels/scalar_temporal_benchmark.cc index 780e90c087e..bd90bbb6a58 100644 --- a/cpp/src/arrow/compute/kernels/scalar_temporal_benchmark.cc +++ b/cpp/src/arrow/compute/kernels/scalar_temporal_benchmark.cc @@ -59,7 +59,7 @@ static void BenchmarkTemporalRounding(benchmark::State& state) { EXPECT_OK_AND_ASSIGN(auto timestamp_array, array->View(timestamp_type)); for (auto _ : state) { - ABORT_NOT_OK(Op(timestamp_array, options, ctx).status()); + ABORT_NOT_OK(Op(timestamp_array, options, ctx)); } state.SetItemsProcessed(state.iterations() * array_size); @@ -78,7 +78,7 @@ static void BenchmarkTemporal(benchmark::State& state) { EXPECT_OK_AND_ASSIGN(auto timestamp_array, array->View(timestamp_type)); for (auto _ : state) { - ABORT_NOT_OK(Op(timestamp_array, ctx).status()); + ABORT_NOT_OK(Op(timestamp_array, ctx)); } state.SetItemsProcessed(state.iterations() * array_size); @@ -96,7 +96,7 @@ static void BenchmarkTemporalBinary(benchmark::State& state) { auto rhs = rand.ArrayOf(timestamp_type, args.size, args.null_proportion); for (auto _ : state) { - ABORT_NOT_OK(Op(lhs, rhs, ctx).status()); + ABORT_NOT_OK(Op(lhs, rhs, ctx)); } state.SetItemsProcessed(state.iterations() * array_size); @@ -116,7 +116,7 @@ static void BenchmarkStrftime(benchmark::State& state) { auto options = StrftimeOptions(); for (auto _ : state) { - ABORT_NOT_OK(Strftime(timestamp_array, options, ctx).status()); + ABORT_NOT_OK(Strftime(timestamp_array, options, ctx)); } state.SetItemsProcessed(state.iterations() * array_size); @@ -139,7 +139,7 @@ static void BenchmarkStrptime(benchmark::State& state) { auto strptime_options = StrptimeOptions("%Y-%m-%dT%H:%M:%S", TimeUnit::MICRO, true); for (auto _ : state) { - ABORT_NOT_OK(Strptime(string_array, strptime_options, ctx).status()); + ABORT_NOT_OK(Strptime(string_array, strptime_options, ctx)); } state.SetItemsProcessed(state.iterations() * array_size); @@ -160,7 +160,7 @@ static void BenchmarkAssumeTimezone(benchmark::State& state) { "Pacific/Marquesas", AssumeTimezoneOptions::Ambiguous::AMBIGUOUS_LATEST, AssumeTimezoneOptions::Nonexistent::NONEXISTENT_EARLIEST); for (auto _ : state) { - ABORT_NOT_OK(AssumeTimezone(timestamp_array, options, ctx).status()); + ABORT_NOT_OK(AssumeTimezone(timestamp_array, options, ctx)); } state.SetItemsProcessed(state.iterations() * array_size); diff --git a/cpp/src/arrow/compute/kernels/vector_hash_benchmark.cc b/cpp/src/arrow/compute/kernels/vector_hash_benchmark.cc index 3200f7a469a..3f8683b74d9 100644 --- a/cpp/src/arrow/compute/kernels/vector_hash_benchmark.cc +++ b/cpp/src/arrow/compute/kernels/vector_hash_benchmark.cc @@ -48,7 +48,7 @@ static void BuildDictionary(benchmark::State& state) { // NOLINT non-const refe ArrayFromVector(is_valid, values, &arr); while (state.KeepRunning()) { - ABORT_NOT_OK(DictionaryEncode(arr).status()); + ABORT_NOT_OK(DictionaryEncode(arr)); } state.counters["null_percent"] = static_cast(arr->null_count()) / arr->length() * 100; @@ -75,7 +75,7 @@ static void BuildStringDictionary( ArrayFromVector(data, &arr); while (state.KeepRunning()) { - ABORT_NOT_OK(DictionaryEncode(arr).status()); + ABORT_NOT_OK(DictionaryEncode(arr)); } state.SetBytesProcessed(state.iterations() * total_bytes); state.SetItemsProcessed(state.iterations() * data.size()); @@ -133,7 +133,7 @@ void BenchUnique(benchmark::State& state, const ParamType& params) { params.GenerateTestData(&arr); while (state.KeepRunning()) { - ABORT_NOT_OK(Unique(arr).status()); + ABORT_NOT_OK(Unique(arr)); } params.SetMetadata(state); } @@ -143,7 +143,7 @@ void BenchDictionaryEncode(benchmark::State& state, const ParamType& params) { std::shared_ptr arr; params.GenerateTestData(&arr); while (state.KeepRunning()) { - ABORT_NOT_OK(DictionaryEncode(arr).status()); + ABORT_NOT_OK(DictionaryEncode(arr)); } params.SetMetadata(state); } @@ -215,7 +215,7 @@ void BenchValueCountsDictionaryChunks(benchmark::State& state, const ParamType& auto chunked_array = std::make_shared(chunks); while (state.KeepRunning()) { - ABORT_NOT_OK(ValueCounts(chunked_array).status()); + ABORT_NOT_OK(ValueCounts(chunked_array)); } params.SetMetadata(state); } diff --git a/cpp/src/arrow/compute/kernels/vector_pairwise.cc b/cpp/src/arrow/compute/kernels/vector_pairwise.cc index e7c92d78a07..e5a1eb3c88c 100644 --- a/cpp/src/arrow/compute/kernels/vector_pairwise.cc +++ b/cpp/src/arrow/compute/kernels/vector_pairwise.cc @@ -147,7 +147,7 @@ void RegisterPairwiseDiffKernels(std::string_view func_name, doc, GetDefaultPairwiseOptions()); auto base_func_result = registry->GetFunction(std::string(base_func_name)); - DCHECK_OK(base_func_result.status()); + DCHECK_OK(base_func_result); const auto& base_func = checked_cast(**base_func_result); DCHECK_EQ(base_func.arity().num_args, 2); diff --git a/cpp/src/arrow/compute/kernels/vector_partition_benchmark.cc b/cpp/src/arrow/compute/kernels/vector_partition_benchmark.cc index 7f199b25b76..7d0418e029d 100644 --- a/cpp/src/arrow/compute/kernels/vector_partition_benchmark.cc +++ b/cpp/src/arrow/compute/kernels/vector_partition_benchmark.cc @@ -31,7 +31,7 @@ constexpr auto kSeed = 0x0ff1ce; static void NthToIndicesBenchmark(benchmark::State& state, const std::shared_ptr& values, int64_t n) { for (auto _ : state) { - ABORT_NOT_OK(NthToIndices(*values, n).status()); + ABORT_NOT_OK(NthToIndices(*values, n)); } state.SetItemsProcessed(state.iterations() * values->length()); } diff --git a/cpp/src/arrow/compute/kernels/vector_selection_benchmark.cc b/cpp/src/arrow/compute/kernels/vector_selection_benchmark.cc index 040dfc9656d..b9c8480d988 100644 --- a/cpp/src/arrow/compute/kernels/vector_selection_benchmark.cc +++ b/cpp/src/arrow/compute/kernels/vector_selection_benchmark.cc @@ -211,7 +211,7 @@ struct TakeBenchmark { } for (auto _ : state) { - ABORT_NOT_OK(Take(values, indices).status()); + ABORT_NOT_OK(Take(values, indices)); } state.SetItemsProcessed(state.iterations() * num_indices); state.counters["selection_factor"] = selection_factor; @@ -253,11 +253,11 @@ struct TakeBenchmark { if (chunk_indices_too) { for (auto _ : state) { - ABORT_NOT_OK(Take(values, chunked_indices).status()); + ABORT_NOT_OK(Take(values, chunked_indices)); } } else { for (auto _ : state) { - ABORT_NOT_OK(Take(values, indices).status()); + ABORT_NOT_OK(Take(values, indices)); } } state.SetItemsProcessed(state.iterations() * num_indices); @@ -321,7 +321,7 @@ struct FilterBenchmark { auto filter = rand.Boolean(values->length(), args.selected_proportion, args.filter_null_proportion); for (auto _ : state) { - ABORT_NOT_OK(Filter(values, filter).status()); + ABORT_NOT_OK(Filter(values, filter)); } state.SetItemsProcessed(state.iterations() * values->length()); } @@ -356,7 +356,7 @@ struct FilterBenchmark { auto batch = RecordBatch::Make(schema(fields), num_rows, columns); for (auto _ : state) { - ABORT_NOT_OK(Filter(batch, filter).status()); + ABORT_NOT_OK(Filter(batch, filter)); } state.SetItemsProcessed(state.iterations() * num_rows); } diff --git a/cpp/src/arrow/compute/kernels/vector_sort_benchmark.cc b/cpp/src/arrow/compute/kernels/vector_sort_benchmark.cc index 5c31d4da38a..787b32b6363 100644 --- a/cpp/src/arrow/compute/kernels/vector_sort_benchmark.cc +++ b/cpp/src/arrow/compute/kernels/vector_sort_benchmark.cc @@ -309,7 +309,7 @@ static void ChunkedArraySortIndicesString(benchmark::State& state) { static void DatumSortIndicesBenchmark(benchmark::State& state, const Datum& datum, const SortOptions& options) { for (auto _ : state) { - ABORT_NOT_OK(SortIndices(datum, options).status()); + ABORT_NOT_OK(SortIndices(datum, options)); } } diff --git a/cpp/src/arrow/compute/kernels/vector_topk_benchmark.cc b/cpp/src/arrow/compute/kernels/vector_topk_benchmark.cc index 44452471b25..f0248d0d8d2 100644 --- a/cpp/src/arrow/compute/kernels/vector_topk_benchmark.cc +++ b/cpp/src/arrow/compute/kernels/vector_topk_benchmark.cc @@ -33,7 +33,7 @@ constexpr auto kSeed = 0x0ff1ce; static void SelectKBenchmark(benchmark::State& state, const std::shared_ptr& values, int64_t k) { for (auto _ : state) { - ABORT_NOT_OK(SelectKUnstable(*values, SelectKOptions::TopKDefault(k)).status()); + ABORT_NOT_OK(SelectKUnstable(*values, SelectKOptions::TopKDefault(k))); } state.SetItemsProcessed(state.iterations() * values->length()); } diff --git a/cpp/src/arrow/dataset/dataset_writer.cc b/cpp/src/arrow/dataset/dataset_writer.cc index 7845f488219..a7807e30a66 100644 --- a/cpp/src/arrow/dataset/dataset_writer.cc +++ b/cpp/src/arrow/dataset/dataset_writer.cc @@ -227,11 +227,8 @@ class DatasetWriterFileQueue Status Finish() { writer_state_->staged_rows_count -= rows_currently_staged_; while (!staged_batches_.empty()) { - auto st = PopAndDeliverStagedBatch().status(); - if (!st.ok()) { - file_tasks_.reset(); - return st; - } + RETURN_NOT_OK(PopAndDeliverStagedBatch().status().OrElse( + [&](auto&&) { file_tasks_.reset(); })); } // At this point all write tasks have been added. Because the scheduler // is a 1-task FIFO we know this task will run at the very end and can diff --git a/cpp/src/arrow/filesystem/azurefs_test.cc b/cpp/src/arrow/filesystem/azurefs_test.cc index 31deb42ce0a..12395f2dedc 100644 --- a/cpp/src/arrow/filesystem/azurefs_test.cc +++ b/cpp/src/arrow/filesystem/azurefs_test.cc @@ -1094,7 +1094,7 @@ class TestAzureFileSystem : public ::testing::Test { } #define ASSERT_RAISES_ERRNO(expr, expected_errno) \ - for (::arrow::Status _st = ::arrow::internal::GenericToStatus((expr)); \ + for (::arrow::Status _st = ::arrow::ToStatus((expr)); \ !WithErrno(_st, (expected_errno));) \ FAIL() << "'" ARROW_STRINGIFY(expr) "' did not fail with errno=" << #expected_errno \ << ": " << _st.ToString() @@ -1872,7 +1872,7 @@ class TestAzureFileSystem : public ::testing::Test { FileInfo _src_info; \ ASSERT_OK( \ CheckExpectedErrno(_src, _dest, _expected_errno, #expected_errno, &_src_info)); \ - auto _move_st = ::arrow::internal::GenericToStatus(fs()->Move(_src, _dest)); \ + auto _move_st = ::arrow::ToStatus(fs()->Move(_src, _dest)); \ if (_expected_errno.has_value()) { \ if (WithErrno(_move_st, *_expected_errno)) { \ /* If the Move failed, the source should remain unchanged. */ \ diff --git a/cpp/src/arrow/gpu/cuda_memory.cc b/cpp/src/arrow/gpu/cuda_memory.cc index 9d71678ff3b..735a5dacc2b 100644 --- a/cpp/src/arrow/gpu/cuda_memory.cc +++ b/cpp/src/arrow/gpu/cuda_memory.cc @@ -206,7 +206,7 @@ CudaHostBuffer::CudaHostBuffer(uint8_t* data, const int64_t size) CudaHostBuffer::~CudaHostBuffer() { auto maybe_manager = CudaDeviceManager::Instance(); - ARROW_CHECK_OK(maybe_manager.status()); + ARROW_CHECK_OK(maybe_manager); ARROW_CHECK_OK((*maybe_manager)->FreeHost(const_cast(data_), size_)); } diff --git a/cpp/src/arrow/io/file_test.cc b/cpp/src/arrow/io/file_test.cc index 44a63e9fdfa..81ae716ef67 100644 --- a/cpp/src/arrow/io/file_test.cc +++ b/cpp/src/arrow/io/file_test.cc @@ -434,7 +434,7 @@ TEST_F(TestReadableFile, NonexistentFile) { auto maybe_file = ReadableFile::Open(path); ASSERT_RAISES(IOError, maybe_file); std::string message = maybe_file.status().message(); - ASSERT_NE(std::string::npos, message.find(path)); + ASSERT_NE(std::string::npos, message.find(path)) << message; } class MyMemoryPool : public MemoryPool { diff --git a/cpp/src/arrow/io/memory_test.cc b/cpp/src/arrow/io/memory_test.cc index 03d0e65daee..eabee87146d 100644 --- a/cpp/src/arrow/io/memory_test.cc +++ b/cpp/src/arrow/io/memory_test.cc @@ -681,7 +681,7 @@ TEST(TestInputStreamIterator, Closed) { AssertBufferEqual(*buf, "dat"); // Close stream and read from iterator ASSERT_OK(reader->Close()); - ASSERT_RAISES(Invalid, it.Next().status()); + ASSERT_RAISES(Invalid, it.Next()); } TEST(CoalesceReadRanges, Basics) { diff --git a/cpp/src/arrow/record_batch.h b/cpp/src/arrow/record_batch.h index a3cd4103853..43a8ee63b59 100644 --- a/cpp/src/arrow/record_batch.h +++ b/cpp/src/arrow/record_batch.h @@ -390,7 +390,7 @@ class ARROW_EXPORT RecordBatchReader { } Result> operator*() { - ARROW_RETURN_NOT_OK(batch_.status()); + ARROW_RETURN_NOT_OK(batch_); return batch_; } diff --git a/cpp/src/arrow/result.h b/cpp/src/arrow/result.h index 3c4f98c7f1f..895f3085c62 100644 --- a/cpp/src/arrow/result.h +++ b/cpp/src/arrow/result.h @@ -489,19 +489,11 @@ class [[nodiscard]] Result : public util::EqualityComparable> { ARROW_ASSIGN_OR_RAISE_IMPL(ARROW_ASSIGN_OR_RAISE_NAME(_error_or_value, __COUNTER__), \ lhs, rexpr); -namespace internal { - -template -inline const Status& GenericToStatus(const Result& res) { - return res.status(); -} - template -inline Status GenericToStatus(Result&& res) { - return std::move(res).status(); -} - -} // namespace internal +struct IntoStatus> { + static constexpr const Status& ToStatus(const Result& res) { return res.status(); } + static inline Status ToStatus(Result&& res) { return std::move(res).status(); } +}; template ::type> R ToResult(T t) { diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index 6938bc0d887..0f770f15b11 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -795,8 +795,8 @@ TEST(TestFixedSizeBinaryScalar, MakeScalar) { AssertParseScalar(type, std::string_view(data), FixedSizeBinaryScalar(buf, type)); // Wrong length - ASSERT_RAISES(Invalid, MakeScalar(type, Buffer::FromString(data.substr(3))).status()); - ASSERT_RAISES(Invalid, Scalar::Parse(type, std::string_view(data).substr(3)).status()); + ASSERT_RAISES(Invalid, MakeScalar(type, Buffer::FromString(data.substr(3)))); + ASSERT_RAISES(Invalid, Scalar::Parse(type, std::string_view(data).substr(3))); } TEST(TestFixedSizeBinaryScalar, ValidateErrors) { @@ -1438,13 +1438,13 @@ TEST(TestStructScalar, FieldAccess) { ASSERT_OK_AND_ASSIGN(auto a, abc.field("a")); AssertScalarsEqual(*a, *abc.value[0]); - ASSERT_RAISES(Invalid, abc.field("b").status()); + ASSERT_RAISES(Invalid, abc.field("b")); ASSERT_OK_AND_ASSIGN(auto b, abc.field(1)); AssertScalarsEqual(*b, *abc.value[1]); - ASSERT_RAISES(Invalid, abc.field(5).status()); - ASSERT_RAISES(Invalid, abc.field("c").status()); + ASSERT_RAISES(Invalid, abc.field(5)); + ASSERT_RAISES(Invalid, abc.field("c")); ASSERT_OK_AND_ASSIGN(auto d, abc.field("d")); ASSERT_TRUE(d->Equals(*MakeNullScalar(int64()))); diff --git a/cpp/src/arrow/status.h b/cpp/src/arrow/status.h index 42e8929ce0b..596f1cc1d18 100644 --- a/cpp/src/arrow/status.h +++ b/cpp/src/arrow/status.h @@ -18,6 +18,7 @@ #include #include #include +#include #include #include "arrow/util/compare.h" @@ -52,10 +53,10 @@ ARROW_RETURN_IF_(condition, status, ARROW_STRINGIFY(status)) /// \brief Propagate any non-successful Status to the caller -#define ARROW_RETURN_NOT_OK(status) \ - do { \ - ::arrow::Status __s = ::arrow::internal::GenericToStatus(status); \ - ARROW_RETURN_IF_(!__s.ok(), __s, ARROW_STRINGIFY(status)); \ +#define ARROW_RETURN_NOT_OK(status) \ + do { \ + ::arrow::Status __s = ::arrow::ToStatus(status); \ + ARROW_RETURN_IF_(!__s.ok(), __s, ARROW_STRINGIFY(status)); \ } while (false) /// \brief Given `expr` and `warn_msg`; log `warn_msg` if `expr` is a non-ok status @@ -67,15 +68,6 @@ } \ } while (false) -#define RETURN_NOT_OK_ELSE(s, else_) \ - do { \ - ::arrow::Status _s = ::arrow::internal::GenericToStatus(s); \ - if (!_s.ok()) { \ - else_; \ - return _s; \ - } \ - } while (false) - // This is an internal-use macro and should not be used in public headers. #ifndef RETURN_NOT_OK # define RETURN_NOT_OK(s) ARROW_RETURN_NOT_OK(s) @@ -83,8 +75,10 @@ namespace arrow { namespace internal { + class StatusConstant; -} + +} // namespace internal enum class StatusCode : char { OK = 0, @@ -124,6 +118,23 @@ class ARROW_EXPORT StatusDetail { } }; +/// \brief A type trait to declare a given type as Status-compatible. +/// +/// This trait structure can be implemented if a type (such as Result) embeds +/// error information that can be converted to the Status class. +/// It will make the given type usable directly in functions such as +/// Status::OrElse and error-checking macros such as ARROW_RETURN_NOT_OK. +template +struct IntoStatus; + +/// \brief Convert a Status-compatible object to Status +/// +/// This generic function delegates to the IntoStatus type trait. +template +constexpr decltype(auto) ToStatus(T&& t) { + return IntoStatus>::ToStatus(std::forward(t)); +} + /// \brief Status outcome object (success or error) /// /// The Status object is an object holding the outcome of an operation. @@ -350,6 +361,32 @@ class ARROW_EXPORT [[nodiscard]] Status : public util::EqualityComparable(args)...).WithDetail(detail()); } + /// \brief Apply a functor if the status indicates an error + /// + /// This can be used to execute fallback or cleanup actions. + /// + /// If the status indicates a success, it is returned as-is. + /// + /// If the status indicates an error, the given functor is called with the status + /// as argument. + /// If the functor returns a new Status, it is returned. + /// If the functor returns a Status-compatible object such as Result, it is + /// converted to Status and returned. + /// If the functor returns void, the original Status is returned. + template + Status OrElse(OnError&& on_error) { + using RT = decltype(on_error(Status())); + if (ARROW_PREDICT_TRUE(ok())) { + return *this; + } + if constexpr (std::is_void_v) { + on_error(*this); + return *this; + } else { + return ToStatus(on_error(*this)); + } + } + void Warn() const; void Warn(const std::string& message) const; @@ -463,13 +500,10 @@ Status& Status::operator&=(Status&& s) noexcept { } /// \endcond -namespace internal { - -// Extract Status from Status or Result -// Useful for the status check macros such as RETURN_NOT_OK. -inline const Status& GenericToStatus(const Status& st) { return st; } -inline Status GenericToStatus(Status&& st) { return std::move(st); } - -} // namespace internal +template <> +struct IntoStatus { + static constexpr const Status& ToStatus(const Status& st) { return st; } + static constexpr Status&& ToStatus(Status&& st) { return std::move(st); } +}; } // namespace arrow diff --git a/cpp/src/arrow/status_test.cc b/cpp/src/arrow/status_test.cc index 005bdf665f5..39a52bd2bad 100644 --- a/cpp/src/arrow/status_test.cc +++ b/cpp/src/arrow/status_test.cc @@ -20,6 +20,7 @@ #include #include +#include "arrow/result.h" #include "arrow/status.h" #include "arrow/status_internal.h" #include "arrow/testing/gtest_util.h" @@ -37,6 +38,25 @@ class TestStatusDetail : public StatusDetail { } // namespace +namespace my_namespace { + +struct StatusLike { + int value; // ok if 42 +}; + +} // namespace my_namespace + +template <> +struct IntoStatus { + static inline Status ToStatus(my_namespace::StatusLike v) { + if (v.value == 42) { + return Status::OK(); + } else { + return Status::UnknownError("StatusLike: ", v.value); + } + } +}; + TEST(StatusTest, TestCodeAndMessage) { Status ok = Status::OK(); ASSERT_EQ(StatusCode::OK, ok.code()); @@ -234,4 +254,92 @@ TEST(StatusTest, TestDetailEquality) { ASSERT_NE(status_without_detail, status_with_detail); } +TEST(StatusTest, OrElse) { + int called = 0; + + auto or_else_returning_status = [&](Status st) { + ++called; + return st.WithMessage("Prefixed: ", st.message()); + }; + auto or_else_returning_result = [&](Status st) { + ++called; + return Result(st.WithMessage("Prefixed: ", st.message())); + }; + auto or_else_returning_user_class = [&](Status st) { + ++called; + return my_namespace::StatusLike{43}; + }; + auto or_else_returning_void = [&](auto) { ++called; }; + + auto ok_status = Status::OK(); + auto error_status = Status::IOError("some message"); + Status st; + + st = ok_status.OrElse(or_else_returning_status); + ASSERT_TRUE(st.ok()); + st = ok_status.OrElse(or_else_returning_result); + ASSERT_TRUE(st.ok()); + st = ok_status.OrElse(or_else_returning_void); + ASSERT_TRUE(st.ok()); + st = ok_status.OrElse(or_else_returning_user_class); + ASSERT_TRUE(st.ok()); + ASSERT_EQ(called, 0); + + st = error_status.OrElse(or_else_returning_status); + ASSERT_EQ(st.code(), StatusCode::IOError); + ASSERT_EQ(st.message(), "Prefixed: some message"); + ASSERT_EQ(called, 1); + st = error_status.OrElse(or_else_returning_result); + ASSERT_EQ(st.code(), StatusCode::IOError); + ASSERT_EQ(st.message(), "Prefixed: some message"); + ASSERT_EQ(called, 2); + st = error_status.OrElse(or_else_returning_void); + ASSERT_EQ(st.code(), StatusCode::IOError); + ASSERT_EQ(st.message(), "some message"); + ASSERT_EQ(called, 3); + st = error_status.OrElse(or_else_returning_user_class); + ASSERT_EQ(st.code(), StatusCode::UnknownError); + ASSERT_EQ(st.message(), "StatusLike: 43"); + ASSERT_EQ(called, 4); +} + +std::string StripContext(const std::string& message) { +#ifdef ARROW_EXTRA_ERROR_CONTEXT + auto pos = message.find_first_of('\n'); + if (pos != message.npos) { + return message.substr(0, pos); + } +#endif + return message; +} + +TEST(StatusTest, ReturnIfNotOk) { + auto f = [](auto v) { + RETURN_NOT_OK(v); + return Status::OK(); + }; + + auto ok_status = Status::OK(); + auto error_status = Status::IOError("some message"); + Status st; + + st = f(ok_status); + ASSERT_TRUE(st.ok()); + st = f(error_status); + ASSERT_EQ(st.code(), StatusCode::IOError); + ASSERT_EQ(StripContext(st.message()), error_status.message()); + + st = f(Result(42)); + ASSERT_TRUE(st.ok()); + st = f(Result(error_status)); + ASSERT_EQ(st.code(), StatusCode::IOError); + ASSERT_EQ(StripContext(st.message()), error_status.message()); + + st = f(my_namespace::StatusLike{42}); + ASSERT_TRUE(st.ok()); + st = f(my_namespace::StatusLike{43}); + ASSERT_EQ(st.code(), StatusCode::UnknownError); + ASSERT_EQ(StripContext(st.message()), "StatusLike: 43"); +} + } // namespace arrow diff --git a/cpp/src/arrow/testing/gtest_util.h b/cpp/src/arrow/testing/gtest_util.h index 005610ea3fc..8e537bd26de 100644 --- a/cpp/src/arrow/testing/gtest_util.h +++ b/cpp/src/arrow/testing/gtest_util.h @@ -49,8 +49,7 @@ // NOTE: using a for loop for this macro allows extra failure messages to be // appended with operator<< #define ASSERT_RAISES(ENUM, expr) \ - for (::arrow::Status _st = ::arrow::internal::GenericToStatus((expr)); \ - !_st.Is##ENUM();) \ + for (::arrow::Status _st = ::arrow::ToStatus((expr)); !_st.Is##ENUM();) \ FAIL() << "Expected '" ARROW_STRINGIFY(expr) "' to fail with " ARROW_STRINGIFY( \ ENUM) ", but got " \ << _st.ToString() @@ -58,7 +57,7 @@ #define ASSERT_RAISES_WITH_MESSAGE(ENUM, message, expr) \ do { \ auto _res = (expr); \ - ::arrow::Status _st = ::arrow::internal::GenericToStatus(_res); \ + ::arrow::Status _st = ::arrow::ToStatus(_res); \ if (!_st.Is##ENUM()) { \ FAIL() << "Expected '" ARROW_STRINGIFY(expr) "' to fail with " ARROW_STRINGIFY( \ ENUM) ", but got " \ @@ -70,7 +69,7 @@ #define EXPECT_RAISES_WITH_MESSAGE_THAT(ENUM, matcher, expr) \ do { \ auto _res = (expr); \ - ::arrow::Status _st = ::arrow::internal::GenericToStatus(_res); \ + ::arrow::Status _st = ::arrow::ToStatus(_res); \ EXPECT_TRUE(_st.Is##ENUM()) << "Expected '" ARROW_STRINGIFY(expr) "' to fail with " \ << ARROW_STRINGIFY(ENUM) ", but got " << _st.ToString(); \ EXPECT_THAT(_st.ToStringWithoutContextLines(), (matcher)); \ @@ -79,13 +78,13 @@ #define EXPECT_RAISES_WITH_CODE_AND_MESSAGE_THAT(code, matcher, expr) \ do { \ auto _res = (expr); \ - ::arrow::Status _st = ::arrow::internal::GenericToStatus(_res); \ + ::arrow::Status _st = ::arrow::ToStatus(_res); \ EXPECT_EQ(_st.CodeAsString(), Status::CodeAsString(code)); \ EXPECT_THAT(_st.ToStringWithoutContextLines(), (matcher)); \ } while (false) -#define ASSERT_OK(expr) \ - for (::arrow::Status _st = ::arrow::internal::GenericToStatus((expr)); !_st.ok();) \ +#define ASSERT_OK(expr) \ + for (::arrow::Status _st = ::arrow::ToStatus((expr)); !_st.ok();) \ FAIL() << "'" ARROW_STRINGIFY(expr) "' failed with " << _st.ToString() #define ASSERT_OK_NO_THROW(expr) ASSERT_NO_THROW(ASSERT_OK(expr)) @@ -93,7 +92,7 @@ #define ARROW_EXPECT_OK(expr) \ do { \ auto _res = (expr); \ - ::arrow::Status _st = ::arrow::internal::GenericToStatus(_res); \ + ::arrow::Status _st = ::arrow::ToStatus(_res); \ EXPECT_TRUE(_st.ok()) << "'" ARROW_STRINGIFY(expr) "' failed with " \ << _st.ToString(); \ } while (false) @@ -102,17 +101,17 @@ #define EXPECT_OK_NO_THROW(expr) EXPECT_NO_THROW(EXPECT_OK(expr)) -#define ASSERT_NOT_OK(expr) \ - for (::arrow::Status _st = ::arrow::internal::GenericToStatus((expr)); _st.ok();) \ +#define ASSERT_NOT_OK(expr) \ + for (::arrow::Status _st = ::arrow::ToStatus((expr)); _st.ok();) \ FAIL() << "'" ARROW_STRINGIFY(expr) "' did not failed" << _st.ToString() -#define ABORT_NOT_OK(expr) \ - do { \ - auto _res = (expr); \ - ::arrow::Status _st = ::arrow::internal::GenericToStatus(_res); \ - if (ARROW_PREDICT_FALSE(!_st.ok())) { \ - _st.Abort(); \ - } \ +#define ABORT_NOT_OK(expr) \ + do { \ + auto _res = (expr); \ + ::arrow::Status _st = ::arrow::ToStatus(_res); \ + if (ARROW_PREDICT_FALSE(!_st.ok())) { \ + _st.Abort(); \ + } \ } while (false); #define ASSIGN_OR_HANDLE_ERROR_IMPL(handle_error, status_name, lhs, rexpr) \ diff --git a/cpp/src/arrow/testing/matchers.h b/cpp/src/arrow/testing/matchers.h index b800cb30c3c..0e1bae47381 100644 --- a/cpp/src/arrow/testing/matchers.h +++ b/cpp/src/arrow/testing/matchers.h @@ -250,7 +250,7 @@ class ErrorMatcher { bool MatchAndExplain(const Res& maybe_value, testing::MatchResultListener* listener) const override { - const Status& status = internal::GenericToStatus(maybe_value); + const Status& status = ToStatus(maybe_value); testing::StringMatchResultListener value_listener; bool match = status.code() == code_; @@ -294,7 +294,7 @@ class OkMatcher { bool MatchAndExplain(const Res& maybe_value, testing::MatchResultListener* listener) const override { - const Status& status = internal::GenericToStatus(maybe_value); + const Status& status = ToStatus(maybe_value); const bool match = status.ok(); *listener << "whose " << (match ? "non-error matches" : "error doesn't match"); diff --git a/cpp/src/arrow/type.cc b/cpp/src/arrow/type.cc index 221071b4f0a..99e51cdbe23 100644 --- a/cpp/src/arrow/type.cc +++ b/cpp/src/arrow/type.cc @@ -2150,7 +2150,7 @@ std::vector FieldRef::FindAll(const FieldVector& fields) const { auto maybe_field = FieldPathGetImpl::Get(&path, FieldSelector(fields_), &out_of_range_depth); - DCHECK_OK(maybe_field.status()); + DCHECK_OK(maybe_field); if (maybe_field.ValueOrDie() != nullptr) { return {path}; @@ -2188,7 +2188,7 @@ std::vector FieldRef::FindAll(const FieldVector& fields) const { void Add(const FieldPath& prefix, const FieldPath& suffix, const FieldVector& fields) { auto maybe_field = suffix.Get(fields); - DCHECK_OK(maybe_field.status()); + DCHECK_OK(maybe_field); referents.push_back(std::move(maybe_field).ValueOrDie()); std::vector concatenated_indices(prefix.indices().size() + diff --git a/cpp/src/arrow/util/cancel.cc b/cpp/src/arrow/util/cancel.cc index 81a16dbe228..a83475fed0b 100644 --- a/cpp/src/arrow/util/cancel.cc +++ b/cpp/src/arrow/util/cancel.cc @@ -169,7 +169,7 @@ struct SignalStopState : public std::enable_shared_from_this { self_pipe_ptr_.store(nullptr); auto handlers = std::move(saved_handlers_); for (const auto& h : handlers) { - ARROW_CHECK_OK(SetSignalHandler(h.signum, h.handler).status()); + ARROW_CHECK_OK(SetSignalHandler(h.signum, h.handler)); } } diff --git a/cpp/src/arrow/util/decimal.cc b/cpp/src/arrow/util/decimal.cc index 39c1fb067c6..457a51a714b 100644 --- a/cpp/src/arrow/util/decimal.cc +++ b/cpp/src/arrow/util/decimal.cc @@ -45,6 +45,29 @@ using internal::SafeLeftShift; using internal::SafeSignedAdd; using internal::uint128_t; +namespace internal { + +Status ToArrowStatus(DecimalStatus dstatus) { + switch (dstatus) { + case DecimalStatus::kSuccess: + return Status::OK(); + + case DecimalStatus::kDivideByZero: + return Status::Invalid("Division by 0 in Decimal"); + + case DecimalStatus::kOverflow: + return Status::Invalid("Overflow occurred during Decimal operation"); + + case DecimalStatus::kRescaleDataLoss: + return Status::Invalid("Rescaling Decimal value would cause data loss"); + + default: + return Status::UnknownError("Unknown Decimal error"); + } +} + +} // namespace internal + namespace { struct BaseDecimalRealConversion { @@ -835,24 +858,6 @@ bool ParseDecimalComponents(const char* s, size_t size, DecimalComponents* out) return pos == size; } -inline Status ToArrowStatus(DecimalStatus dstatus, int num_bits) { - switch (dstatus) { - case DecimalStatus::kSuccess: - return Status::OK(); - - case DecimalStatus::kDivideByZero: - return Status::Invalid("Division by 0 in Decimal", num_bits); - - case DecimalStatus::kOverflow: - return Status::Invalid("Overflow occurred during Decimal", num_bits, " operation."); - - case DecimalStatus::kRescaleDataLoss: - return Status::Invalid("Rescaling Decimal", num_bits, - " value would cause data loss"); - } - return Status::OK(); -} - template Status DecimalFromString(const char* type_name, std::string_view s, Decimal* out, int32_t* precision, int32_t* scale) { @@ -1105,10 +1110,6 @@ Result Decimal32::FromBigEndian(const uint8_t* bytes, int32_t length) return Decimal32(value); } -Status Decimal32::ToArrowStatus(DecimalStatus dstatus) const { - return arrow::ToArrowStatus(dstatus, 32); -} - std::ostream& operator<<(std::ostream& os, const Decimal32& decimal) { os << decimal.ToIntegerString(); return os; @@ -1132,10 +1133,6 @@ Result Decimal64::FromBigEndian(const uint8_t* bytes, int32_t length) return Decimal64(value); } -Status Decimal64::ToArrowStatus(DecimalStatus dstatus) const { - return arrow::ToArrowStatus(dstatus, 64); -} - std::ostream& operator<<(std::ostream& os, const Decimal64& decimal) { os << decimal.ToIntegerString(); return os; @@ -1194,10 +1191,6 @@ Result Decimal128::FromBigEndian(const uint8_t* bytes, int32_t lengt return Decimal128(high, static_cast(low)); } -Status Decimal128::ToArrowStatus(DecimalStatus dstatus) const { - return arrow::ToArrowStatus(dstatus, 128); -} - std::ostream& operator<<(std::ostream& os, const Decimal128& decimal) { os << decimal.ToIntegerString(); return os; @@ -1302,10 +1295,6 @@ Result Decimal256::FromBigEndian(const uint8_t* bytes, int32_t lengt return Decimal256(bit_util::little_endian::ToNative(little_endian_array)); } -Status Decimal256::ToArrowStatus(DecimalStatus dstatus) const { - return arrow::ToArrowStatus(dstatus, 256); -} - namespace { struct Decimal256RealConversion diff --git a/cpp/src/arrow/util/decimal.h b/cpp/src/arrow/util/decimal.h index 00328668928..bae0c4dd248 100644 --- a/cpp/src/arrow/util/decimal.h +++ b/cpp/src/arrow/util/decimal.h @@ -33,6 +33,18 @@ namespace arrow { class Decimal64; +namespace internal { + +ARROW_EXPORT +Status ToArrowStatus(DecimalStatus); + +} // namespace internal + +template <> +struct IntoStatus { + static inline Status ToStatus(DecimalStatus st) { return internal::ToArrowStatus(st); } +}; + /// Represents a signed 32-bit decimal value in two's complement. /// Calulations wrap around and overflow is ignored. /// The max decimal precision that can be safely represented is @@ -75,8 +87,7 @@ class ARROW_EXPORT Decimal32 : public BasicDecimal32 { /// \return the pair of the quotient and the remainder Result> Divide(const Decimal32& divisor) const { std::pair result; - auto dstatus = BasicDecimal32::Divide(divisor, &result.first, &result.second); - ARROW_RETURN_NOT_OK(ToArrowStatus(dstatus)); + ARROW_RETURN_NOT_OK(BasicDecimal32::Divide(divisor, &result.first, &result.second)); return result; } @@ -114,8 +125,7 @@ class ARROW_EXPORT Decimal32 : public BasicDecimal32 { /// \brief Convert Decimal32 from one scale to another Result Rescale(int32_t original_scale, int32_t new_scale) const { Decimal32 out; - auto dstatus = BasicDecimal32::Rescale(original_scale, new_scale, &out); - ARROW_RETURN_NOT_OK(ToArrowStatus(dstatus)); + ARROW_RETURN_NOT_OK(BasicDecimal32::Rescale(original_scale, new_scale, &out)); return out; } @@ -150,10 +160,6 @@ class ARROW_EXPORT Decimal32 : public BasicDecimal32 { ARROW_FRIEND_EXPORT friend std::ostream& operator<<(std::ostream& os, const Decimal32& decimal); - - private: - /// Converts internal error code to Status - Status ToArrowStatus(DecimalStatus dstatus) const; }; class ARROW_EXPORT Decimal64 : public BasicDecimal64 { @@ -189,8 +195,7 @@ class ARROW_EXPORT Decimal64 : public BasicDecimal64 { /// \return the pair of the quotient and the remainder Result> Divide(const Decimal64& divisor) const { std::pair result; - auto dstatus = BasicDecimal64::Divide(divisor, &result.first, &result.second); - ARROW_RETURN_NOT_OK(ToArrowStatus(dstatus)); + ARROW_RETURN_NOT_OK(BasicDecimal64::Divide(divisor, &result.first, &result.second)); return result; } @@ -226,8 +231,7 @@ class ARROW_EXPORT Decimal64 : public BasicDecimal64 { /// \brief Convert Decimal64 from one scale to another Result Rescale(int32_t original_scale, int32_t new_scale) const { Decimal64 out; - auto dstatus = BasicDecimal64::Rescale(original_scale, new_scale, &out); - ARROW_RETURN_NOT_OK(ToArrowStatus(dstatus)); + ARROW_RETURN_NOT_OK(BasicDecimal64::Rescale(original_scale, new_scale, &out)); return out; } @@ -262,10 +266,6 @@ class ARROW_EXPORT Decimal64 : public BasicDecimal64 { ARROW_FRIEND_EXPORT friend std::ostream& operator<<(std::ostream& os, const Decimal64& decimal); - - private: - /// Converts internal error code to Status - Status ToArrowStatus(DecimalStatus dstatus) const; }; /// Represents a signed 128-bit integer in two's complement. @@ -315,8 +315,7 @@ class ARROW_EXPORT Decimal128 : public BasicDecimal128 { /// \return the pair of the quotient and the remainder Result> Divide(const Decimal128& divisor) const { std::pair result; - auto dstatus = BasicDecimal128::Divide(divisor, &result.first, &result.second); - ARROW_RETURN_NOT_OK(ToArrowStatus(dstatus)); + ARROW_RETURN_NOT_OK(BasicDecimal128::Divide(divisor, &result.first, &result.second)); return result; } @@ -353,8 +352,7 @@ class ARROW_EXPORT Decimal128 : public BasicDecimal128 { /// \brief Convert Decimal128 from one scale to another Result Rescale(int32_t original_scale, int32_t new_scale) const { Decimal128 out; - auto dstatus = BasicDecimal128::Rescale(original_scale, new_scale, &out); - ARROW_RETURN_NOT_OK(ToArrowStatus(dstatus)); + ARROW_RETURN_NOT_OK(BasicDecimal128::Rescale(original_scale, new_scale, &out)); return out; } @@ -396,10 +394,6 @@ class ARROW_EXPORT Decimal128 : public BasicDecimal128 { ARROW_FRIEND_EXPORT friend std::ostream& operator<<(std::ostream& os, const Decimal128& decimal); - - private: - /// Converts internal error code to Status - Status ToArrowStatus(DecimalStatus dstatus) const; }; /// Represents a signed 256-bit integer in two's complement. @@ -453,8 +447,7 @@ class ARROW_EXPORT Decimal256 : public BasicDecimal256 { /// \brief Convert Decimal256 from one scale to another Result Rescale(int32_t original_scale, int32_t new_scale) const { Decimal256 out; - auto dstatus = BasicDecimal256::Rescale(original_scale, new_scale, &out); - ARROW_RETURN_NOT_OK(ToArrowStatus(dstatus)); + ARROW_RETURN_NOT_OK(BasicDecimal256::Rescale(original_scale, new_scale, &out)); return out; } @@ -470,8 +463,7 @@ class ARROW_EXPORT Decimal256 : public BasicDecimal256 { /// \return the pair of the quotient and the remainder Result> Divide(const Decimal256& divisor) const { std::pair result; - auto dstatus = BasicDecimal256::Divide(divisor, &result.first, &result.second); - ARROW_RETURN_NOT_OK(ToArrowStatus(dstatus)); + ARROW_RETURN_NOT_OK(BasicDecimal256::Divide(divisor, &result.first, &result.second)); return result; } @@ -503,10 +495,6 @@ class ARROW_EXPORT Decimal256 : public BasicDecimal256 { ARROW_FRIEND_EXPORT friend std::ostream& operator<<(std::ostream& os, const Decimal256& decimal); - - private: - /// Converts internal error code to Status - Status ToArrowStatus(DecimalStatus dstatus) const; }; /// For an integer type, return the max number of decimal digits diff --git a/cpp/src/arrow/util/iterator.h b/cpp/src/arrow/util/iterator.h index d18cb90f042..dc7fd1d84cc 100644 --- a/cpp/src/arrow/util/iterator.h +++ b/cpp/src/arrow/util/iterator.h @@ -165,7 +165,7 @@ class Iterator : public util::EqualityComparable> { } Result operator*() { - ARROW_RETURN_NOT_OK(value_.status()); + ARROW_RETURN_NOT_OK(value_); auto value = std::move(value_); value_ = IterationTraits::End(); diff --git a/cpp/src/arrow/util/iterator_test.cc b/cpp/src/arrow/util/iterator_test.cc index a247ba13aef..64148e58402 100644 --- a/cpp/src/arrow/util/iterator_test.cc +++ b/cpp/src/arrow/util/iterator_test.cc @@ -350,10 +350,10 @@ TEST(TestFunctionIterator, RangeForLoop) { int expected_i = 0; for (auto maybe_i : fails_at_3) { if (expected_i < 3) { - ASSERT_OK(maybe_i.status()); + ASSERT_OK(maybe_i); ASSERT_EQ(*maybe_i, expected_i); } else if (expected_i == 3) { - ASSERT_RAISES(IndexError, maybe_i.status()); + ASSERT_RAISES(IndexError, maybe_i); } ASSERT_LE(expected_i, 3) << "iteration stops after an error is encountered"; ++expected_i; @@ -499,7 +499,7 @@ TEST(ReadaheadIterator, NextError) { ASSERT_OK_AND_ASSIGN( auto it, MakeReadaheadIterator(Iterator(std::move(tracing_it)), 2)); - ASSERT_RAISES(IOError, it.Next().status()); + ASSERT_RAISES(IOError, it.Next()); AssertIteratorExhausted(it); SleepABit(); diff --git a/cpp/src/arrow/util/logging.h b/cpp/src/arrow/util/logging.h index e8c24892aab..460888f6d75 100644 --- a/cpp/src/arrow/util/logging.h +++ b/cpp/src/arrow/util/logging.h @@ -70,7 +70,7 @@ enum class ArrowLogLevel : int { // of 'msg' followed by the status. # define ARROW_CHECK_OK_PREPEND(to_call, msg, level) \ do { \ - ::arrow::Status _s = (to_call); \ + ::arrow::Status _s = ::arrow::ToStatus(to_call); \ ARROW_CHECK_OR_LOG(_s.ok(), level) \ << "Operation failed: " << ARROW_STRINGIFY(to_call) << "\n" \ << (msg) << ": " << _s.ToString(); \ diff --git a/cpp/src/parquet/arrow/writer.cc b/cpp/src/parquet/arrow/writer.cc index 5307465caf1..b40a32eaf8d 100644 --- a/cpp/src/parquet/arrow/writer.cc +++ b/cpp/src/parquet/arrow/writer.cc @@ -388,15 +388,14 @@ class FileWriterImpl : public FileWriter { if (table.num_rows() == 0) { // Append a row group with 0 rows - RETURN_NOT_OK_ELSE(WriteRowGroup(0, 0), PARQUET_IGNORE_NOT_OK(Close())); - return Status::OK(); + RETURN_NOT_OK( + WriteRowGroup(0, 0).OrElse([&](auto&&) { PARQUET_IGNORE_NOT_OK(Close()); })); } for (int chunk = 0; chunk * chunk_size < table.num_rows(); chunk++) { int64_t offset = chunk * chunk_size; - RETURN_NOT_OK_ELSE( - WriteRowGroup(offset, std::min(chunk_size, table.num_rows() - offset)), - PARQUET_IGNORE_NOT_OK(Close())); + RETURN_NOT_OK(WriteRowGroup(offset, std::min(chunk_size, table.num_rows() - offset)) + .OrElse([&](auto&&) { PARQUET_IGNORE_NOT_OK(Close()); })); } return Status::OK(); } diff --git a/cpp/src/parquet/exception.h b/cpp/src/parquet/exception.h index cd221ec7a24..416e42683a1 100644 --- a/cpp/src/parquet/exception.h +++ b/cpp/src/parquet/exception.h @@ -59,18 +59,18 @@ // Arrow Status to Parquet exception -#define PARQUET_IGNORE_NOT_OK(s) \ - do { \ - ::arrow::Status _s = ::arrow::internal::GenericToStatus(s); \ - ARROW_UNUSED(_s); \ +#define PARQUET_IGNORE_NOT_OK(s) \ + do { \ + ::arrow::Status _s = ::arrow::ToStatus(s); \ + ARROW_UNUSED(_s); \ } while (0) -#define PARQUET_THROW_NOT_OK(s) \ - do { \ - ::arrow::Status _s = ::arrow::internal::GenericToStatus(s); \ - if (!_s.ok()) { \ - throw ::parquet::ParquetStatusException(std::move(_s)); \ - } \ +#define PARQUET_THROW_NOT_OK(s) \ + do { \ + ::arrow::Status _s = ::arrow::ToStatus(s); \ + if (!_s.ok()) { \ + throw ::parquet::ParquetStatusException(std::move(_s)); \ + } \ } while (0) #define PARQUET_ASSIGN_OR_THROW_IMPL(status_name, lhs, rexpr) \ diff --git a/python/pyarrow/src/arrow/python/common.h b/python/pyarrow/src/arrow/python/common.h index 4a7886695ea..affefe2859b 100644 --- a/python/pyarrow/src/arrow/python/common.h +++ b/python/pyarrow/src/arrow/python/common.h @@ -158,8 +158,7 @@ auto SafeCallIntoPython(Function&& func) -> decltype(func()) { auto maybe_status = std::forward(func)(); // If the return Status is a "Python error", the current Python error status // describes the error and shouldn't be clobbered. - if (!IsPyError(::arrow::internal::GenericToStatus(maybe_status)) && - exc_type != NULLPTR) { + if (!IsPyError(::arrow::ToStatus(maybe_status)) && exc_type != NULLPTR) { PyErr_Restore(exc_type, exc_value, exc_traceback); } return maybe_status; diff --git a/python/pyarrow/src/arrow/python/python_test.cc b/python/pyarrow/src/arrow/python/python_test.cc index f988f8da31c..f2f9c4791db 100644 --- a/python/pyarrow/src/arrow/python/python_test.cc +++ b/python/pyarrow/src/arrow/python/python_test.cc @@ -94,18 +94,17 @@ } \ } -#define ASSERT_OK(expr) \ - { \ - for (::arrow::Status _st = ::arrow::internal::GenericToStatus((expr)); !_st.ok();) \ - return Status::Invalid("`", #expr, "` failed with ", _st.ToString()); \ +#define ASSERT_OK(expr) \ + { \ + for (::arrow::Status _st = ::arrow::ToStatus((expr)); !_st.ok();) \ + return Status::Invalid("`", #expr, "` failed with ", _st.ToString()); \ } -#define ASSERT_RAISES(code, expr) \ - { \ - for (::arrow::Status _st_expr = ::arrow::internal::GenericToStatus((expr)); \ - !_st_expr.Is##code();) \ - return Status::Invalid("Expected `", #expr, "` to fail with ", #code, \ - ", but got ", _st_expr.ToString()); \ +#define ASSERT_RAISES(code, expr) \ + { \ + for (::arrow::Status _st_expr = ::arrow::ToStatus((expr)); !_st_expr.Is##code();) \ + return Status::Invalid("Expected `", #expr, "` to fail with ", #code, \ + ", but got ", _st_expr.ToString()); \ } namespace arrow { diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py index 0cd76c700bb..009ab1e849b 100644 --- a/python/pyarrow/tests/test_array.py +++ b/python/pyarrow/tests/test_array.py @@ -1820,7 +1820,7 @@ def test_decimal_to_int_non_integer(): for case in non_integer_cases: # test safe casting raises - msg_regexp = 'Rescaling Decimal128 value would cause data loss' + msg_regexp = 'Rescaling Decimal value would cause data loss' with pytest.raises(pa.ArrowInvalid, match=msg_regexp): _check_cast_case(case) @@ -1839,7 +1839,7 @@ def test_decimal_to_decimal(): ) assert result.equals(expected) - msg_regexp = 'Rescaling Decimal128 value would cause data loss' + msg_regexp = 'Rescaling Decimal value would cause data loss' with pytest.raises(pa.ArrowInvalid, match=msg_regexp): result = arr.cast(pa.decimal128(9, 1))