From 5c72f15f99b5f1dd2f6872294bb1d99f0c696995 Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Tue, 3 Jun 2025 10:56:41 -0700 Subject: [PATCH 01/15] Use result pattern for all helpers --- .../arrow/from_json_string_example.cc | 14 ++--- cpp/src/arrow/json/from_string.cc | 45 +++++++------- cpp/src/arrow/json/from_string.h | 52 +++++++--------- cpp/src/arrow/json/from_string_test.cc | 60 +++++++++---------- cpp/src/arrow/testing/gtest_util.cc | 14 ++--- python/pyarrow/src/arrow/python/gdb.cc | 13 ++-- 6 files changed, 92 insertions(+), 106 deletions(-) diff --git a/cpp/examples/arrow/from_json_string_example.cc b/cpp/examples/arrow/from_json_string_example.cc index da13d913489..eb919303fee 100644 --- a/cpp/examples/arrow/from_json_string_example.cc +++ b/cpp/examples/arrow/from_json_string_example.cc @@ -68,15 +68,15 @@ arrow::Status RunExample() { "[[11, 22], null, [null, 33]]")); // ChunkedArrayFromJSONString - std::shared_ptr chunked_array; - ARROW_RETURN_NOT_OK(ChunkedArrayFromJSONString( - arrow::int32(), {"[5, 10]", "[null]", "[16]"}, &chunked_array)); + ARROW_ASSIGN_OR_RAISE( + auto chunked_array, + ChunkedArrayFromJSONString(arrow::int32(), {"[5, 10]", "[null]", "[16]"})); // DictArrayFromJSONString - std::shared_ptr dict_array; - ARROW_RETURN_NOT_OK(DictArrayFromJSONString( - dictionary(arrow::int32(), arrow::utf8()), "[0, 1, 0, 2, 0, 3]", - R"(["k1", "k2", "k3", "k4"])", &dict_array)); + ARROW_ASSIGN_OR_RAISE( + auto dict_array, + DictArrayFromJSONString(dictionary(arrow::int32(), arrow::utf8()), + "[0, 1, 0, 2, 0, 3]", R"(["k1", "k2", "k3", "k4"])")); return arrow::Status::OK(); } diff --git a/cpp/src/arrow/json/from_string.cc b/cpp/src/arrow/json/from_string.cc index b2972f7150e..407db7abb87 100644 --- a/cpp/src/arrow/json/from_string.cc +++ b/cpp/src/arrow/json/from_string.cc @@ -1004,23 +1004,22 @@ Result> ArrayFromJSONString(const std::shared_ptr& type, - const std::vector& json_strings, - std::shared_ptr* out) { +Result> ChunkedArrayFromJSONString( + const std::shared_ptr& type, const std::vector& json_strings) { ArrayVector out_chunks; out_chunks.reserve(json_strings.size()); for (const std::string& chunk_json : json_strings) { out_chunks.emplace_back(); ARROW_ASSIGN_OR_RAISE(out_chunks.back(), ArrayFromJSONString(type, chunk_json)); } - *out = std::make_shared(std::move(out_chunks), type); - return Status::OK(); + std::shared_ptr out = + std::make_shared(std::move(out_chunks), type); + return out; } -Status DictArrayFromJSONString(const std::shared_ptr& type, - std::string_view indices_json, - std::string_view dictionary_json, - std::shared_ptr* out) { +Result> DictArrayFromJSONString( + const std::shared_ptr& type, std::string_view indices_json, + std::string_view dictionary_json) { if (type->id() != Type::DICTIONARY) { return Status::TypeError("DictArrayFromJSON requires dictionary type, got ", *type); } @@ -1031,13 +1030,13 @@ Status DictArrayFromJSONString(const std::shared_ptr& type, ArrayFromJSONString(dictionary_type.index_type(), indices_json)); ARROW_ASSIGN_OR_RAISE(auto dictionary, ArrayFromJSONString(dictionary_type.value_type(), dictionary_json)); - - return DictionaryArray::FromArrays(type, std::move(indices), std::move(dictionary)) - .Value(out); + ARROW_ASSIGN_OR_RAISE(auto out, DictionaryArray::FromArrays(type, std::move(indices), + std::move(dictionary))); + return out; } -Status ScalarFromJSONString(const std::shared_ptr& type, - std::string_view json_string, std::shared_ptr* out) { +Result> ScalarFromJSONString( + const std::shared_ptr& type, std::string_view json_string) { std::shared_ptr converter; RETURN_NOT_OK(GetConverter(type, &converter)); @@ -1052,13 +1051,13 @@ Status ScalarFromJSONString(const std::shared_ptr& type, RETURN_NOT_OK(converter->AppendValue(json_doc)); RETURN_NOT_OK(converter->Finish(&array)); DCHECK_EQ(array->length(), 1); - return array->GetScalar(0).Value(out); + ARROW_ASSIGN_OR_RAISE(auto out, array->GetScalar(0)); + return out; } -Status DictScalarFromJSONString(const std::shared_ptr& type, - std::string_view index_json, - std::string_view dictionary_json, - std::shared_ptr* out) { +Result> DictScalarFromJSONString( + const std::shared_ptr& type, std::string_view index_json, + std::string_view dictionary_json) { if (type->id() != Type::DICTIONARY) { return Status::TypeError("DictScalarFromJSONString requires dictionary type, got ", *type); @@ -1066,14 +1065,14 @@ Status DictScalarFromJSONString(const std::shared_ptr& type, const auto& dictionary_type = checked_cast(*type); - std::shared_ptr index; std::shared_ptr dictionary; - RETURN_NOT_OK(ScalarFromJSONString(dictionary_type.index_type(), index_json, &index)); + ARROW_ASSIGN_OR_RAISE(auto index, + ScalarFromJSONString(dictionary_type.index_type(), index_json)); ARROW_ASSIGN_OR_RAISE( dictionary, ArrayFromJSONString(dictionary_type.value_type(), dictionary_json)); - *out = DictionaryScalar::Make(std::move(index), std::move(dictionary)); - return Status::OK(); + auto out = DictionaryScalar::Make(std::move(index), std::move(dictionary)); + return out; } } // namespace json diff --git a/cpp/src/arrow/json/from_string.h b/cpp/src/arrow/json/from_string.h index 03c6b1bcdf4..50f640f0280 100644 --- a/cpp/src/arrow/json/from_string.h +++ b/cpp/src/arrow/json/from_string.h @@ -48,7 +48,7 @@ namespace json { /// /// \code {.cpp} /// std::shared_ptr array = ArrayFromJSONString( -/// int64(), "[2, 3, null, 7, 11]" +/// int64(), "[2, 3, null, 7, 11]" /// ).ValueOrDie(); /// \endcode ARROW_EXPORT @@ -68,52 +68,46 @@ Result> ArrayFromJSONString(const std::shared_ptr chunked_array; -/// ChunkedArrayFromJSONString( -/// int64(), {R"([5, 10])", R"([null])", R"([16])"}, &chunked_array -/// ); +/// std::shared_ptr chunked_array = +/// ChunkedArrayFromJSONString(int64(), {R"([5, 10])", R"([null])", R"([16])"}) +/// .ValueOrDie(); /// \endcode ARROW_EXPORT -Status ChunkedArrayFromJSONString(const std::shared_ptr& type, - const std::vector& json_strings, - std::shared_ptr* out); +Result> ChunkedArrayFromJSONString( + const std::shared_ptr& type, const std::vector& json_strings); /// \brief Create a DictionaryArray from a JSON string /// /// \code {.cpp} -/// std::shared_ptr array; -/// DictArrayFromJSONString( -/// dictionary(int32(), utf8()), -/// "[0, 1, 0, 2, 0, 3]", R"(["k1", "k2", "k3", "k4"])", -/// &array -/// ); +/// std::shared_ptr dict_array = +/// DictArrayFromJSONString(dictionary(int32(), utf8()), "[0, 1, 0, 2, 0, 3]", +/// R"(["k1", "k2", "k3", "k4"])"); /// \endcode ARROW_EXPORT -Status DictArrayFromJSONString(const std::shared_ptr&, - std::string_view indices_json, - std::string_view dictionary_json, - std::shared_ptr* out); +ARROW_EXPORT +Result> DictArrayFromJSONString(const std::shared_ptr&, + std::string_view indices_json, + std::string_view dictionary_json); /// \brief Create a Scalar from a JSON string /// \code {.cpp} -/// std::shared_ptr scalar; -/// ScalarFromJSONString(float64(), "42", &scalar); +/// std::shared_ptr scalar = +/// ScalarFromJSONString(float64(), "42", &scalar); /// \endcode ARROW_EXPORT -Status ScalarFromJSONString(const std::shared_ptr&, std::string_view json, - std::shared_ptr* out); +Result> ScalarFromJSONString(const std::shared_ptr&, + std::string_view json); /// \brief Create a DictionaryScalar from a JSON string /// \code {.cpp} -/// std::shared_ptr scalar; -/// DictScalarFromJSONString(dictionary(int32(), utf8()), "3", R"(["k1", "k2", "k3", -/// "k4"])", &scalar); +/// std::shared_ptr dict_scalar = +/// DictScalarFromJSONString(dictionary(int32(), utf8()), "3", R"(["k1", "k2", "k3", +/// "k4"])", &scalar); /// \endcode ARROW_EXPORT -Status DictScalarFromJSONString(const std::shared_ptr&, - std::string_view index_json, - std::string_view dictionary_json, - std::shared_ptr* out); +Result> DictScalarFromJSONString( + const std::shared_ptr&, std::string_view index_json, + std::string_view dictionary_json); /// @} diff --git a/cpp/src/arrow/json/from_string_test.cc b/cpp/src/arrow/json/from_string_test.cc index d9fa53f68cb..5c13b4e37eb 100644 --- a/cpp/src/arrow/json/from_string_test.cc +++ b/cpp/src/arrow/json/from_string_test.cc @@ -149,9 +149,9 @@ template void AssertJSONScalar(const std::shared_ptr& type, const std::string& json, const bool is_valid, const C_TYPE value) { SCOPED_TRACE(json); - std::shared_ptr actual, expected; + std::shared_ptr expected; - ASSERT_OK(ScalarFromJSONString(type, json, &actual)); + ASSERT_OK_AND_ASSIGN(auto actual, ScalarFromJSONString(type, json)); if (is_valid) { ASSERT_OK_AND_ASSIGN(expected, MakeScalar(type, value)); } else { @@ -1471,30 +1471,29 @@ TEST(TestDictArrayFromJSON, Basics) { TEST(TestDictArrayFromJSON, Errors) { auto type = dictionary(int32(), utf8()); - std::shared_ptr array; - ASSERT_RAISES(Invalid, DictArrayFromJSONString(type, "[\"not a valid index\"]", - "[\"\"]", &array)); - ASSERT_RAISES(Invalid, DictArrayFromJSONString(type, "[0, 1]", "[1]", - &array)); // dict value isn't string + ASSERT_RAISES(Invalid, + DictArrayFromJSONString(type, "[\"not a valid index\"]", "[\"\"]")); + ASSERT_RAISES(Invalid, DictArrayFromJSONString(type, "[0, 1]", + "[1]")); // dict value isn't string } TEST(TestChunkedArrayFromJSON, Basics) { auto type = int32(); - std::shared_ptr chunked_array; - ASSERT_OK(ChunkedArrayFromJSONString(type, {}, &chunked_array)); + ASSERT_OK_AND_ASSIGN(auto chunked_array, ChunkedArrayFromJSONString(type, {})); ASSERT_OK(chunked_array->ValidateFull()); ASSERT_EQ(chunked_array->num_chunks(), 0); AssertTypeEqual(type, chunked_array->type()); - ASSERT_OK(ChunkedArrayFromJSONString(type, {"[1, 2]", "[3, null, 4]"}, &chunked_array)); - ASSERT_OK(chunked_array->ValidateFull()); - ASSERT_EQ(chunked_array->num_chunks(), 2); + ASSERT_OK_AND_ASSIGN(auto chunked_array_two, + ChunkedArrayFromJSONString(type, {"[1, 2]", "[3, null, 4]"})); + ASSERT_OK(chunked_array_two->ValidateFull()); + ASSERT_EQ(chunked_array_two->num_chunks(), 2); std::shared_ptr expected_chunk; ASSERT_OK_AND_ASSIGN(expected_chunk, ArrayFromJSONString(type, "[1, 2]")); - AssertArraysEqual(*expected_chunk, *chunked_array->chunk(0), /*verbose=*/true); + AssertArraysEqual(*expected_chunk, *chunked_array_two->chunk(0), /*verbose=*/true); ASSERT_OK_AND_ASSIGN(expected_chunk, ArrayFromJSONString(type, "[3, null, 4]")); - AssertArraysEqual(*expected_chunk, *chunked_array->chunk(1), /*verbose=*/true); + AssertArraysEqual(*expected_chunk, *chunked_array_two->chunk(1), /*verbose=*/true); } TEST(TestScalarFromJSON, Basics) { @@ -1516,25 +1515,23 @@ TEST(TestScalarFromJSON, Basics) { AssertJSONScalar(boolean(), "1", true, true); AssertJSONScalar(float64(), "1.0", true, 1.0); AssertJSONScalar(float64(), "-0.0", true, -0.0); - ASSERT_OK(ScalarFromJSONString(float64(), "NaN", &scalar)); + ASSERT_OK(ScalarFromJSONString(float64(), "NaN")); ASSERT_TRUE(std::isnan(checked_cast(*scalar).value)); - ASSERT_OK(ScalarFromJSONString(float64(), "Inf", &scalar)); + ASSERT_OK(ScalarFromJSONString(float64(), "Inf")); ASSERT_TRUE(std::isinf(checked_cast(*scalar).value)); } TEST(TestScalarFromJSON, Errors) { std::shared_ptr scalar; - ASSERT_RAISES(Invalid, ScalarFromJSONString(int64(), "[0]", &scalar)); - ASSERT_RAISES(Invalid, ScalarFromJSONString(int64(), "[9223372036854775808]", &scalar)); - ASSERT_RAISES(Invalid, - ScalarFromJSONString(int64(), "[-9223372036854775809]", &scalar)); - ASSERT_RAISES(Invalid, - ScalarFromJSONString(uint64(), "[18446744073709551616]", &scalar)); - ASSERT_RAISES(Invalid, ScalarFromJSONString(uint64(), "[-1]", &scalar)); - ASSERT_RAISES(Invalid, ScalarFromJSONString(binary(), "0", &scalar)); - ASSERT_RAISES(Invalid, ScalarFromJSONString(binary(), "[]", &scalar)); - ASSERT_RAISES(Invalid, ScalarFromJSONString(boolean(), "0.0", &scalar)); - ASSERT_RAISES(Invalid, ScalarFromJSONString(boolean(), "\"true\"", &scalar)); + ASSERT_RAISES(Invalid, ScalarFromJSONString(int64(), "[0]")); + ASSERT_RAISES(Invalid, ScalarFromJSONString(int64(), "[9223372036854775808]")); + ASSERT_RAISES(Invalid, ScalarFromJSONString(int64(), "[-9223372036854775809]")); + ASSERT_RAISES(Invalid, ScalarFromJSONString(uint64(), "[18446744073709551616]")); + ASSERT_RAISES(Invalid, ScalarFromJSONString(uint64(), "[-1]")); + ASSERT_RAISES(Invalid, ScalarFromJSONString(binary(), "0")); + ASSERT_RAISES(Invalid, ScalarFromJSONString(binary(), "[]")); + ASSERT_RAISES(Invalid, ScalarFromJSONString(boolean(), "0.0")); + ASSERT_RAISES(Invalid, ScalarFromJSONString(boolean(), "\"true\"")); } TEST(TestDictScalarFromJSONString, Basics) { @@ -1553,12 +1550,11 @@ TEST(TestDictScalarFromJSONString, Basics) { TEST(TestDictScalarFromJSONString, Errors) { auto type = dictionary(int32(), utf8()); - std::shared_ptr scalar; - ASSERT_RAISES(Invalid, DictScalarFromJSONString(type, "\"not a valid index\"", "[\"\"]", - &scalar)); - ASSERT_RAISES(Invalid, DictScalarFromJSONString(type, "0", "[1]", - &scalar)); // dict value isn't string + ASSERT_RAISES(Invalid, + DictScalarFromJSONString(type, "\"not a valid index\"", "[\"\"]")); + ASSERT_RAISES(Invalid, + DictScalarFromJSONString(type, "0", "[1]")); // dict value isn't string } } // namespace json diff --git a/cpp/src/arrow/testing/gtest_util.cc b/cpp/src/arrow/testing/gtest_util.cc index 99d6cabde9e..09e42d6f0fb 100644 --- a/cpp/src/arrow/testing/gtest_util.cc +++ b/cpp/src/arrow/testing/gtest_util.cc @@ -387,15 +387,14 @@ std::shared_ptr ArrayFromJSON(const std::shared_ptr& type, std::shared_ptr DictArrayFromJSON(const std::shared_ptr& type, std::string_view indices_json, std::string_view dictionary_json) { - std::shared_ptr out; - ABORT_NOT_OK(json::DictArrayFromJSONString(type, indices_json, dictionary_json, &out)); + EXPECT_OK_AND_ASSIGN( + auto out, json::DictArrayFromJSONString(type, indices_json, dictionary_json)); return out; } std::shared_ptr ChunkedArrayFromJSON(const std::shared_ptr& type, const std::vector& json) { - std::shared_ptr out; - ABORT_NOT_OK(json::ChunkedArrayFromJSONString(type, json, &out)); + EXPECT_OK_AND_ASSIGN(auto out, json::ChunkedArrayFromJSONString(type, json)); return out; } @@ -411,16 +410,15 @@ std::shared_ptr RecordBatchFromJSON(const std::shared_ptr& std::shared_ptr ScalarFromJSON(const std::shared_ptr& type, std::string_view json) { - std::shared_ptr out; - ABORT_NOT_OK(json::ScalarFromJSONString(type, json, &out)); + EXPECT_OK_AND_ASSIGN(auto out, json::ScalarFromJSONString(type, json)); return out; } std::shared_ptr DictScalarFromJSON(const std::shared_ptr& type, std::string_view index_json, std::string_view dictionary_json) { - std::shared_ptr out; - ABORT_NOT_OK(json::DictScalarFromJSONString(type, index_json, dictionary_json, &out)); + EXPECT_OK_AND_ASSIGN(auto out, + json::DictScalarFromJSONString(type, index_json, dictionary_json)); return out; } diff --git a/python/pyarrow/src/arrow/python/gdb.cc b/python/pyarrow/src/arrow/python/gdb.cc index 38383b86f49..7865e8756ac 100644 --- a/python/pyarrow/src/arrow/python/gdb.cc +++ b/python/pyarrow/src/arrow/python/gdb.cc @@ -479,13 +479,12 @@ void TestSession() { key_value_metadata({"key1", "key2", "key3"}, {"value1", "value2", "value3"})); // Table - ChunkedArrayVector table_columns{2}; - ARROW_CHECK_OK( - ChunkedArrayFromJSONString(int32(), {"[1, 2, 3]", "[4, 5]"}, &table_columns[0])); - ARROW_CHECK_OK(ChunkedArrayFromJSONString( - utf8(), {R"(["abc", null])", R"(["def"])", R"(["ghi", "jkl"])"}, - &table_columns[1])); - auto table = Table::Make(batch_schema, table_columns); + ASSERT_OK_AND_ASSIGN(auto col1, + ChunkedArrayFromJSONString(int32(), {"[1, 2, 3]", "[4, 5]"})); + ASSERT_OK_AND_ASSIGN( + auto col2, ChunkedArrayFromJSONString( + utf8(), {R"(["abc", null])", R"(["def"])", R"(["ghi", "jkl"])"})); + auto table = Table::Make(batch_schema, {col1, col2}); // Datum Datum empty_datum{}; From d451933e608dd1372ba7fc0ae22dabb87b89fd2f Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Tue, 3 Jun 2025 18:56:28 +0000 Subject: [PATCH 02/15] Remove extra ARROW_EXPORT --- cpp/src/arrow/json/from_string.h | 1 - 1 file changed, 1 deletion(-) diff --git a/cpp/src/arrow/json/from_string.h b/cpp/src/arrow/json/from_string.h index 50f640f0280..105327f6bd4 100644 --- a/cpp/src/arrow/json/from_string.h +++ b/cpp/src/arrow/json/from_string.h @@ -84,7 +84,6 @@ Result> ChunkedArrayFromJSONString( /// R"(["k1", "k2", "k3", "k4"])"); /// \endcode ARROW_EXPORT -ARROW_EXPORT Result> DictArrayFromJSONString(const std::shared_ptr&, std::string_view indices_json, std::string_view dictionary_json); From 92ee48a36c0bd2a32a3b69c65894a4f15c41157a Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Tue, 3 Jun 2025 12:53:31 -0700 Subject: [PATCH 03/15] Fix things missed in previous commits --- cpp/src/arrow/json/from_string_test.cc | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/cpp/src/arrow/json/from_string_test.cc b/cpp/src/arrow/json/from_string_test.cc index 5c13b4e37eb..b70501b5f34 100644 --- a/cpp/src/arrow/json/from_string_test.cc +++ b/cpp/src/arrow/json/from_string_test.cc @@ -1498,7 +1498,6 @@ TEST(TestChunkedArrayFromJSON, Basics) { TEST(TestScalarFromJSON, Basics) { // Sanity check for common types (not exhaustive) - std::shared_ptr scalar; AssertJSONScalar(int64(), "4", true, 4); AssertJSONScalar(int64(), "null", false, 0); AssertJSONScalar>(utf8(), R"("")", true, @@ -1515,14 +1514,13 @@ TEST(TestScalarFromJSON, Basics) { AssertJSONScalar(boolean(), "1", true, true); AssertJSONScalar(float64(), "1.0", true, 1.0); AssertJSONScalar(float64(), "-0.0", true, -0.0); - ASSERT_OK(ScalarFromJSONString(float64(), "NaN")); - ASSERT_TRUE(std::isnan(checked_cast(*scalar).value)); - ASSERT_OK(ScalarFromJSONString(float64(), "Inf")); - ASSERT_TRUE(std::isinf(checked_cast(*scalar).value)); + ASSERT_OK_AND_ASSIGN(auto nan_scalar, ScalarFromJSONString(float64(), "NaN")); + ASSERT_TRUE(std::isnan(checked_cast(*nan_scalar).value)); + ASSERT_OK_AND_ASSIGN(auto inf_scalar, ScalarFromJSONString(float64(), "Inf")); + ASSERT_TRUE(std::isinf(checked_cast(*inf_scalar).value)); } TEST(TestScalarFromJSON, Errors) { - std::shared_ptr scalar; ASSERT_RAISES(Invalid, ScalarFromJSONString(int64(), "[0]")); ASSERT_RAISES(Invalid, ScalarFromJSONString(int64(), "[9223372036854775808]")); ASSERT_RAISES(Invalid, ScalarFromJSONString(int64(), "[-9223372036854775809]")); From 95d146a508d96194039a14d0fe4166be80111123 Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Tue, 3 Jun 2025 16:18:26 -0700 Subject: [PATCH 04/15] Update cpp/src/arrow/json/from_string.cc Co-authored-by: Sutou Kouhei --- cpp/src/arrow/json/from_string.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cpp/src/arrow/json/from_string.cc b/cpp/src/arrow/json/from_string.cc index 407db7abb87..da94f5d4554 100644 --- a/cpp/src/arrow/json/from_string.cc +++ b/cpp/src/arrow/json/from_string.cc @@ -1030,9 +1030,7 @@ Result> DictArrayFromJSONString( ArrayFromJSONString(dictionary_type.index_type(), indices_json)); ARROW_ASSIGN_OR_RAISE(auto dictionary, ArrayFromJSONString(dictionary_type.value_type(), dictionary_json)); - ARROW_ASSIGN_OR_RAISE(auto out, DictionaryArray::FromArrays(type, std::move(indices), - std::move(dictionary))); - return out; + return DictionaryArray::FromArrays(type, std::move(indices), std::move(dictionary))); } Result> ScalarFromJSONString( From 141a0f4d48b48fdb0ab46481a0bd4d2869767b8a Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Tue, 3 Jun 2025 16:18:32 -0700 Subject: [PATCH 05/15] Update cpp/src/arrow/json/from_string.cc Co-authored-by: Sutou Kouhei --- cpp/src/arrow/json/from_string.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cpp/src/arrow/json/from_string.cc b/cpp/src/arrow/json/from_string.cc index da94f5d4554..d503c827448 100644 --- a/cpp/src/arrow/json/from_string.cc +++ b/cpp/src/arrow/json/from_string.cc @@ -1069,8 +1069,7 @@ Result> DictScalarFromJSONString( ARROW_ASSIGN_OR_RAISE( dictionary, ArrayFromJSONString(dictionary_type.value_type(), dictionary_json)); - auto out = DictionaryScalar::Make(std::move(index), std::move(dictionary)); - return out; + return DictionaryScalar::Make(std::move(index), std::move(dictionary)); } } // namespace json From d38feaa0ec2d30f7ee40b016502a2843744b80d6 Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Tue, 3 Jun 2025 16:20:24 -0700 Subject: [PATCH 06/15] Update cpp/src/arrow/json/from_string.h Co-authored-by: Sutou Kouhei --- cpp/src/arrow/json/from_string.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cpp/src/arrow/json/from_string.h b/cpp/src/arrow/json/from_string.h index 105327f6bd4..c401ba06c14 100644 --- a/cpp/src/arrow/json/from_string.h +++ b/cpp/src/arrow/json/from_string.h @@ -68,9 +68,8 @@ Result> ArrayFromJSONString(const std::shared_ptr chunked_array = -/// ChunkedArrayFromJSONString(int64(), {R"([5, 10])", R"([null])", R"([16])"}) -/// .ValueOrDie(); +/// Result> chunked_array_result = +/// ChunkedArrayFromJSONString(int64(), {R"([5, 10])", R"([null])", R"([16])"}); /// \endcode ARROW_EXPORT Result> ChunkedArrayFromJSONString( From 8a2240fb3e4e4fd270521ef6ed90bf3c77663fd7 Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Tue, 3 Jun 2025 16:24:17 -0700 Subject: [PATCH 07/15] Fix code doxygen comments --- cpp/src/arrow/json/from_string.h | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/cpp/src/arrow/json/from_string.h b/cpp/src/arrow/json/from_string.h index c401ba06c14..251e087d323 100644 --- a/cpp/src/arrow/json/from_string.h +++ b/cpp/src/arrow/json/from_string.h @@ -47,9 +47,8 @@ namespace json { /// \brief Create an Array from a JSON string /// /// \code {.cpp} -/// std::shared_ptr array = ArrayFromJSONString( -/// int64(), "[2, 3, null, 7, 11]" -/// ).ValueOrDie(); +/// Result> array = +/// ArrayFromJSONString(int64(), "[2, 3, null, 7, 11]"); /// \endcode ARROW_EXPORT Result> ArrayFromJSONString(const std::shared_ptr&, @@ -78,7 +77,7 @@ Result> ChunkedArrayFromJSONString( /// \brief Create a DictionaryArray from a JSON string /// /// \code {.cpp} -/// std::shared_ptr dict_array = +/// Result> dict_array = /// DictArrayFromJSONString(dictionary(int32(), utf8()), "[0, 1, 0, 2, 0, 3]", /// R"(["k1", "k2", "k3", "k4"])"); /// \endcode @@ -89,7 +88,7 @@ Result> DictArrayFromJSONString(const std::shared_ptr scalar = +/// Result> scalar = /// ScalarFromJSONString(float64(), "42", &scalar); /// \endcode ARROW_EXPORT @@ -98,7 +97,7 @@ Result> ScalarFromJSONString(const std::shared_ptr dict_scalar = +/// Result> dict_scalar = /// DictScalarFromJSONString(dictionary(int32(), utf8()), "3", R"(["k1", "k2", "k3", /// "k4"])", &scalar); /// \endcode From 58ff5549f5b9d87aa09c02ba37742cd5f147ea7a Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Tue, 3 Jun 2025 16:27:58 -0700 Subject: [PATCH 08/15] Use same convention for variable names --- cpp/src/arrow/json/from_string.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/json/from_string.h b/cpp/src/arrow/json/from_string.h index 251e087d323..8a0f3c9ab0e 100644 --- a/cpp/src/arrow/json/from_string.h +++ b/cpp/src/arrow/json/from_string.h @@ -67,7 +67,7 @@ Result> ArrayFromJSONString(const std::shared_ptr> chunked_array_result = +/// Result> chunked_array = /// ChunkedArrayFromJSONString(int64(), {R"([5, 10])", R"([null])", R"([16])"}); /// \endcode ARROW_EXPORT From 605328f28a5c5675be54137472d3a1c35e0d637f Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Tue, 3 Jun 2025 17:15:49 -0700 Subject: [PATCH 09/15] Update cpp/src/arrow/json/from_string.cc Co-authored-by: Sutou Kouhei --- cpp/src/arrow/json/from_string.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cpp/src/arrow/json/from_string.cc b/cpp/src/arrow/json/from_string.cc index d503c827448..613ab95b613 100644 --- a/cpp/src/arrow/json/from_string.cc +++ b/cpp/src/arrow/json/from_string.cc @@ -1012,9 +1012,7 @@ Result> ChunkedArrayFromJSONString( out_chunks.emplace_back(); ARROW_ASSIGN_OR_RAISE(out_chunks.back(), ArrayFromJSONString(type, chunk_json)); } - std::shared_ptr out = - std::make_shared(std::move(out_chunks), type); - return out; + return std::make_shared(std::move(out_chunks), type); } Result> DictArrayFromJSONString( From 59bbfe69b7a542d74d777a6c3552141fdbaed524 Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Tue, 3 Jun 2025 17:18:05 -0700 Subject: [PATCH 10/15] Prefix variable names with maybe_ --- cpp/src/arrow/json/from_string.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cpp/src/arrow/json/from_string.h b/cpp/src/arrow/json/from_string.h index 8a0f3c9ab0e..bd5ed3d46a3 100644 --- a/cpp/src/arrow/json/from_string.h +++ b/cpp/src/arrow/json/from_string.h @@ -47,7 +47,7 @@ namespace json { /// \brief Create an Array from a JSON string /// /// \code {.cpp} -/// Result> array = +/// Result> maybe_array = /// ArrayFromJSONString(int64(), "[2, 3, null, 7, 11]"); /// \endcode ARROW_EXPORT @@ -67,7 +67,7 @@ Result> ArrayFromJSONString(const std::shared_ptr> chunked_array = +/// Result> maybe_chunked_array = /// ChunkedArrayFromJSONString(int64(), {R"([5, 10])", R"([null])", R"([16])"}); /// \endcode ARROW_EXPORT @@ -77,7 +77,7 @@ Result> ChunkedArrayFromJSONString( /// \brief Create a DictionaryArray from a JSON string /// /// \code {.cpp} -/// Result> dict_array = +/// Result> maybe_dict_array = /// DictArrayFromJSONString(dictionary(int32(), utf8()), "[0, 1, 0, 2, 0, 3]", /// R"(["k1", "k2", "k3", "k4"])"); /// \endcode @@ -88,7 +88,7 @@ Result> DictArrayFromJSONString(const std::shared_ptr> scalar = +/// Result> maybe_scalar = /// ScalarFromJSONString(float64(), "42", &scalar); /// \endcode ARROW_EXPORT @@ -97,7 +97,7 @@ Result> ScalarFromJSONString(const std::shared_ptr> dict_scalar = +/// Result> maybe_dict_scalar = /// DictScalarFromJSONString(dictionary(int32(), utf8()), "3", R"(["k1", "k2", "k3", /// "k4"])", &scalar); /// \endcode From ed41702796f3d4714078414d371f59ab33373d86 Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Tue, 3 Jun 2025 17:23:00 -0700 Subject: [PATCH 11/15] Fix syntax error --- cpp/src/arrow/json/from_string.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/json/from_string.cc b/cpp/src/arrow/json/from_string.cc index 613ab95b613..fa325eaeeaf 100644 --- a/cpp/src/arrow/json/from_string.cc +++ b/cpp/src/arrow/json/from_string.cc @@ -1028,7 +1028,7 @@ Result> DictArrayFromJSONString( ArrayFromJSONString(dictionary_type.index_type(), indices_json)); ARROW_ASSIGN_OR_RAISE(auto dictionary, ArrayFromJSONString(dictionary_type.value_type(), dictionary_json)); - return DictionaryArray::FromArrays(type, std::move(indices), std::move(dictionary))); + return DictionaryArray::FromArrays(type, std::move(indices), std::move(dictionary)); } Result> ScalarFromJSONString( From 69f3efab97c16c70f3d239b20434aced5f6127d8 Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Tue, 3 Jun 2025 17:50:48 -0700 Subject: [PATCH 12/15] Update cpp/src/arrow/json/from_string.cc Co-authored-by: Sutou Kouhei --- cpp/src/arrow/json/from_string.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cpp/src/arrow/json/from_string.cc b/cpp/src/arrow/json/from_string.cc index fa325eaeeaf..e35a362f5a2 100644 --- a/cpp/src/arrow/json/from_string.cc +++ b/cpp/src/arrow/json/from_string.cc @@ -1047,8 +1047,7 @@ Result> ScalarFromJSONString( RETURN_NOT_OK(converter->AppendValue(json_doc)); RETURN_NOT_OK(converter->Finish(&array)); DCHECK_EQ(array->length(), 1); - ARROW_ASSIGN_OR_RAISE(auto out, array->GetScalar(0)); - return out; + return array->GetScalar(0); } Result> DictScalarFromJSONString( From 165fed627fbb1e719d07fac3e0150f06e4572159 Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Tue, 3 Jun 2025 18:10:37 -0700 Subject: [PATCH 13/15] Fix issue in gdb.cc Not sure how CI didn't catch this. --- python/pyarrow/src/arrow/python/gdb.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/pyarrow/src/arrow/python/gdb.cc b/python/pyarrow/src/arrow/python/gdb.cc index 7865e8756ac..677e5ab680f 100644 --- a/python/pyarrow/src/arrow/python/gdb.cc +++ b/python/pyarrow/src/arrow/python/gdb.cc @@ -363,9 +363,9 @@ void TestSession() { ExtensionScalar extension_scalar_null{extension_scalar.value, extension_scalar_type, /*is_valid=*/false}; - std::shared_ptr heap_map_scalar; - ARROW_CHECK_OK(ScalarFromJSONString(map(utf8(), int32()), R"([["a", 5], ["b", 6]])", - &heap_map_scalar)); + ASSERT_OK_AND_ASSIGN( + auto heap_map_scalar, + ScalarFromJSONString(map(utf8(), int32()), R"([["a", 5], ["b", 6]])")); auto heap_map_scalar_null = MakeNullScalar(heap_map_scalar->type); // Array and ArrayData From 641839ff99f929e845d4ef117e4fa5e7d1a08aa5 Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Tue, 3 Jun 2025 18:49:11 -0700 Subject: [PATCH 14/15] Fix issues in gdb.cc --- python/pyarrow/src/arrow/python/gdb.cc | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/python/pyarrow/src/arrow/python/gdb.cc b/python/pyarrow/src/arrow/python/gdb.cc index 677e5ab680f..a2f96d81902 100644 --- a/python/pyarrow/src/arrow/python/gdb.cc +++ b/python/pyarrow/src/arrow/python/gdb.cc @@ -363,10 +363,9 @@ void TestSession() { ExtensionScalar extension_scalar_null{extension_scalar.value, extension_scalar_type, /*is_valid=*/false}; - ASSERT_OK_AND_ASSIGN( - auto heap_map_scalar, - ScalarFromJSONString(map(utf8(), int32()), R"([["a", 5], ["b", 6]])")); - auto heap_map_scalar_null = MakeNullScalar(heap_map_scalar->type); + auto heap_map_scalar = + ScalarFromJSONString(map(utf8(), int32()), R"([["a", 5], ["b", 6]])"); + auto heap_map_scalar_null = MakeNullScalar(heap_map_scalar.ValueOrDie()->type); // Array and ArrayData auto heap_null_array = SliceArrayFromJSON(null(), "[null, null]"); @@ -479,12 +478,10 @@ void TestSession() { key_value_metadata({"key1", "key2", "key3"}, {"value1", "value2", "value3"})); // Table - ASSERT_OK_AND_ASSIGN(auto col1, - ChunkedArrayFromJSONString(int32(), {"[1, 2, 3]", "[4, 5]"})); - ASSERT_OK_AND_ASSIGN( - auto col2, ChunkedArrayFromJSONString( - utf8(), {R"(["abc", null])", R"(["def"])", R"(["ghi", "jkl"])"})); - auto table = Table::Make(batch_schema, {col1, col2}); + auto col1 = ChunkedArrayFromJSONString(int32(), {"[1, 2, 3]", "[4, 5]"}); + auto col2 = ChunkedArrayFromJSONString( + utf8(), {R"(["abc", null])", R"(["def"])", R"(["ghi", "jkl"])"}); + auto table = Table::Make(batch_schema, {*col1, *col2}); // Datum Datum empty_datum{}; From 7c1c6be59491ab3d1c8ddd4fc685d422e3d1643d Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Tue, 3 Jun 2025 19:26:31 -0700 Subject: [PATCH 15/15] Maybe fix gdb.cc (can't test on macOS) --- python/pyarrow/src/arrow/python/gdb.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/pyarrow/src/arrow/python/gdb.cc b/python/pyarrow/src/arrow/python/gdb.cc index a2f96d81902..2a7d2eda4bf 100644 --- a/python/pyarrow/src/arrow/python/gdb.cc +++ b/python/pyarrow/src/arrow/python/gdb.cc @@ -364,8 +364,8 @@ void TestSession() { /*is_valid=*/false}; auto heap_map_scalar = - ScalarFromJSONString(map(utf8(), int32()), R"([["a", 5], ["b", 6]])"); - auto heap_map_scalar_null = MakeNullScalar(heap_map_scalar.ValueOrDie()->type); + *ScalarFromJSONString(map(utf8(), int32()), R"([["a", 5], ["b", 6]])"); + auto heap_map_scalar_null = MakeNullScalar(heap_map_scalar->type); // Array and ArrayData auto heap_null_array = SliceArrayFromJSON(null(), "[null, null]");