diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_test.cc b/cpp/src/arrow/compute/kernels/scalar_cast_test.cc index 51e6ca534cd5..364a4bd436b6 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_test.cc @@ -31,6 +31,7 @@ #include "arrow/chunked_array.h" #include "arrow/extension_type.h" #include "arrow/status.h" +#include "arrow/testing/builder.h" #include "arrow/testing/extension_type.h" #include "arrow/testing/gtest_util.h" #include "arrow/testing/random.h" @@ -56,16 +57,7 @@ using internal::checked_pointer_cast; namespace compute { static std::shared_ptr InvalidUtf8(std::shared_ptr type) { - return ArrayFromJSON(type, - "[" - R"( - "Hi", - "olá mundo", - "你好世界", - "", - )" - "\"\xa0\xa1\"" - "]"); + return BinaryArrayFromStrings(type, {"Hi", "olá mundo", "你好世界", "", "\xa0\xa1"}); } static std::shared_ptr FixedSizeInvalidUtf8(std::shared_ptr type) { @@ -73,16 +65,7 @@ static std::shared_ptr FixedSizeInvalidUtf8(std::shared_ptr typ // Assume a particular width for testing EXPECT_EQ(3, checked_cast(*type).byte_width()); } - return ArrayFromJSON(type, - "[" - R"( - "Hi!", - "lá", - "你", - " ", - )" - "\"\xa0\xa1\xa2\"" - "]"); + return BinaryArrayFromStrings(type, {"Hi!", "lá", "你", " ", "\xa0\xa1\xa2"}); } static std::vector> kNumericTypes = { diff --git a/cpp/src/arrow/compute/kernels/scalar_string_test.cc b/cpp/src/arrow/compute/kernels/scalar_string_test.cc index b279f991f6ea..b90cb1c80d0a 100644 --- a/cpp/src/arrow/compute/kernels/scalar_string_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_string_test.cc @@ -1102,7 +1102,7 @@ TYPED_TEST(TestStringKernels, Utf8Reverse) { // inputs with malformed utf8 chars would produce garbage output, but the end result // would produce arrays with same lengths. Hence checking offset buffer equality - auto malformed_input = ArrayFromJSON(this->type(), "[\"ɑ\xFFɑa\", \"ɽ\xe1\xbdɽa\"]"); + auto malformed_input = this->MakeArray({"ɑ\xFFɑa", "ɽ\xe1\xbdɽa"}); const Result& res = CallFunction("utf8_reverse", {malformed_input}); ASSERT_TRUE(res->array()->buffers[1]->Equals(*malformed_input->data()->buffers[1])); } @@ -1225,7 +1225,7 @@ TYPED_TEST(TestStringKernels, Utf8Upper) { this->CheckUnary("utf8_upper", "[\"ɑɑɑɑ\"]", this->type(), "[\"ⱭⱭⱭⱭ\"]"); // Test invalid data - auto invalid_input = ArrayFromJSON(this->type(), "[\"ɑa\xFFɑ\", \"ɽ\xe1\xbdɽaa\"]"); + auto invalid_input = this->MakeArray({"ɑa\xFFɑ", "ɽ\xe1\xbdɽaa"}); EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid, testing::HasSubstr("Invalid UTF8 sequence"), CallFunction("utf8_upper", {invalid_input})); } @@ -1247,7 +1247,7 @@ TYPED_TEST(TestStringKernels, Utf8Lower) { this->CheckUnary("utf8_lower", "[\"ȺȺȺȺ\"]", this->type(), "[\"ⱥⱥⱥⱥ\"]"); // Test invalid data - auto invalid_input = ArrayFromJSON(this->type(), "[\"Ⱥa\xFFⱭ\", \"Ɽ\xe1\xbdⱤaA\"]"); + auto invalid_input = this->MakeArray({"Ⱥa\xFFⱭ", "Ɽ\xe1\xbdⱤaA"}); EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid, testing::HasSubstr("Invalid UTF8 sequence"), CallFunction("utf8_lower", {invalid_input})); } @@ -1267,7 +1267,7 @@ TYPED_TEST(TestStringKernels, Utf8SwapCase) { "[\"HeLLo, wOrLD!\", \"$. a35?\"]"); // Test invalid data - auto invalid_input = ArrayFromJSON(this->type(), "[\"Ⱥa\xFFⱭ\", \"Ɽ\xe1\xbdⱤaA\"]"); + auto invalid_input = this->MakeArray({"Ⱥa\xFFⱭ", "Ɽ\xe1\xbdⱤaA"}); EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid, testing::HasSubstr("Invalid UTF8 sequence"), CallFunction("utf8_swapcase", {invalid_input})); } @@ -2516,101 +2516,92 @@ TYPED_TEST(TestBinaryKernels, SliceBytesPosPos) { SliceOptions options{2, 4}; this->CheckUnary( "binary_slice", - "[\"\", \"a\", \"ab\", \"a\xc2\xa2\", \"ab\xc2\xa2\", \"ab\xc2\xffZ\"]", - this->type(), "[\"\", \"\", \"\", \"\xa2\", \"\xc2\xa2\", \"\xc2\xff\"]", &options); + this->MakeArray({"", "a", "ab", "a\xc2\xa2", "ab\xc2\xa2", "ab\xc2\xffZ"}), + this->MakeArray({"", "", "", "\xa2", "\xc2\xa2", "\xc2\xff"}), &options); SliceOptions options_step{1, 5, 2}; this->CheckUnary( "binary_slice", - "[\"\", \"a\", \"ab\", \"a\xc2\xa2\", \"ab\xc2\xa2\", \"ab\xc2\xffZ\"]", - this->type(), "[\"\", \"\", \"b\", \"\xc2\", \"b\xa2\", \"b\xff\"]", &options_step); + this->MakeArray({"", "a", "ab", "a\xc2\xa2", "ab\xc2\xa2", "ab\xc2\xffZ"}), + this->MakeArray({"", "", "b", "\xc2", "b\xa2", "b\xff"}), &options_step); SliceOptions options_step_neg{5, 1, -2}; this->CheckUnary( "binary_slice", - "[\"\", \"a\", \"ab\", \"a\xc2\xa2\", \"ab\xc2\xa2\", \"ab\xc2\xffZ\"]", - this->type(), "[\"\", \"\", \"\", \"\xa2\", \"\xa2\", \"Z\xc2\"]", - &options_step_neg); + this->MakeArray({"", "a", "ab", "a\xc2\xa2", "ab\xc2\xa2", "ab\xc2\xffZ"}), + this->MakeArray({"", "", "", "\xa2", "\xa2", "Z\xc2"}), &options_step_neg); options_step_neg.stop = 0; this->CheckUnary( "binary_slice", - "[\"\", \"a\", \"ab\", \"a\xc2\xa2\", \"aZ\xc2\xa2\", \"ab\xc2\xffZ\"]", - this->type(), "[\"\", \"\", \"b\", \"\xa2\", \"\xa2Z\", \"Z\xc2\"]", - &options_step_neg); + this->MakeArray({"", "a", "ab", "a\xc2\xa2", "aZ\xc2\xa2", "ab\xc2\xffZ"}), + this->MakeArray({"", "", "b", "\xa2", "\xa2Z", "Z\xc2"}), &options_step_neg); } TYPED_TEST(TestBinaryKernels, SliceBytesPosNeg) { SliceOptions options{2, -1}; this->CheckUnary( "binary_slice", - "[\"\", \"a\", \"ab\", \"a\xc2\xa2\", \"aZ\xc2\xa2\", \"ab\xc2\xffZ\"]", - this->type(), "[\"\", \"\", \"\", \"\", \"\xc2\", \"\xc2\xff\"]", &options); + this->MakeArray({"", "a", "ab", "a\xc2\xa2", "aZ\xc2\xa2", "ab\xc2\xffZ"}), + this->MakeArray({"", "", "", "", "\xc2", "\xc2\xff"}), &options); SliceOptions options_step{1, -1, 2}; this->CheckUnary( "binary_slice", - "[\"\", \"a\", \"ab\", \"a\xc2\xa2\", \"aZ\xc2\xa2\", \"ab\xc2\xffZ\"]", - this->type(), "[\"\", \"\", \"\", \"\xc2\", \"Z\", \"b\xff\"]", &options_step); + this->MakeArray({"", "a", "ab", "a\xc2\xa2", "aZ\xc2\xa2", "ab\xc2\xffZ"}), + this->MakeArray({"", "", "", "\xc2", "Z", "b\xff"}), &options_step); SliceOptions options_step_neg{3, -4, -2}; this->CheckUnary( "binary_slice", - "[\"\", \"a\", \"ab\", \"Z\xc2\xa2\", \"aZ\xc2\xa2\", \"ab\xc2\xffZ\"]", - this->type(), "[\"\", \"a\", \"b\", \"\xa2Z\", \"\xa2Z\", \"\xff\"]", - &options_step_neg); + this->MakeArray({"", "a", "ab", "Z\xc2\xa2", "aZ\xc2\xa2", "ab\xc2\xffZ"}), + this->MakeArray({"", "a", "b", "\xa2Z", "\xa2Z", "\xff"}), &options_step_neg); options_step_neg.stop = -5; this->CheckUnary( "binary_slice", - "[\"\", \"a\", \"ab\", \"Z\xc2\xa2\", \"aZ\xc2\xa2\", \"aP\xc2\xffZ\"]", - this->type(), "[\"\", \"a\", \"b\", \"\xa2Z\", \"\xa2Z\", \"\xffP\"]", - &options_step_neg); + this->MakeArray({"", "a", "ab", "Z\xc2\xa2", "aZ\xc2\xa2", "aP\xc2\xffZ"}), + this->MakeArray({"", "a", "b", "\xa2Z", "\xa2Z", "\xffP"}), &options_step_neg); } TYPED_TEST(TestBinaryKernels, SliceBytesNegNeg) { SliceOptions options{-2, -1}; this->CheckUnary( "binary_slice", - "[\"\", \"a\", \"ab\", \"Z\xc2\xa2\", \"aZ\xc2\xa2\", \"ab\xc2\xffZ\"]", - this->type(), "[\"\", \"\", \"a\", \"\xc2\", \"\xc2\", \"\xff\"]", &options); + this->MakeArray({"", "a", "ab", "Z\xc2\xa2", "aZ\xc2\xa2", "ab\xc2\xffZ"}), + this->MakeArray({"", "", "a", "\xc2", "\xc2", "\xff"}), &options); SliceOptions options_step{-4, -1, 2}; this->CheckUnary( "binary_slice", - "[\"\", \"a\", \"ab\", \"Z\xc2\xa2\", \"aZ\xc2\xa2\", \"aP\xc2\xffZ\"]", - this->type(), "[\"\", \"\", \"a\", \"Z\", \"a\xc2\", \"P\xff\"]", &options_step); + this->MakeArray({"", "a", "ab", "Z\xc2\xa2", "aZ\xc2\xa2", "aP\xc2\xffZ"}), + this->MakeArray({"", "", "a", "Z", "a\xc2", "P\xff"}), &options_step); SliceOptions options_step_neg{-1, -3, -2}; this->CheckUnary( "binary_slice", - "[\"\", \"a\", \"ab\", \"Z\xc2\xa2\", \"aZ\xc2\xa2\", \"aP\xc2\xffZ\"]", - this->type(), "[\"\", \"a\", \"b\", \"\xa2\", \"\xa2\", \"Z\"]", &options_step_neg); + this->MakeArray({"", "a", "ab", "Z\xc2\xa2", "aZ\xc2\xa2", "aP\xc2\xffZ"}), + this->MakeArray({"", "a", "b", "\xa2", "\xa2", "Z"}), &options_step_neg); options_step_neg.stop = -4; this->CheckUnary( "binary_slice", - "[\"\", \"a\", \"ab\", \"Z\xc2\xa2\", \"aZ\xc2\xa2\", \"aP\xc2\xffZ\"]", - this->type(), "[\"\", \"a\", \"b\", \"\xa2Z\", \"\xa2Z\", \"Z\xc2\"]", - &options_step_neg); + this->MakeArray({"", "a", "ab", "Z\xc2\xa2", "aZ\xc2\xa2", "aP\xc2\xffZ"}), + this->MakeArray({"", "a", "b", "\xa2Z", "\xa2Z", "Z\xc2"}), &options_step_neg); } TYPED_TEST(TestBinaryKernels, SliceBytesNegPos) { SliceOptions options{-2, 4}; this->CheckUnary( "binary_slice", - "[\"\", \"a\", \"ab\", \"Z\xc2\xa2\", \"aZ\xc2\xa2\", \"aP\xc2\xffZ\"]", - this->type(), "[\"\", \"a\", \"ab\", \"\xc2\xa2\", \"\xc2\xa2\", \"\xff\"]", - &options); + this->MakeArray({"", "a", "ab", "Z\xc2\xa2", "aZ\xc2\xa2", "aP\xc2\xffZ"}), + this->MakeArray({"", "a", "ab", "\xc2\xa2", "\xc2\xa2", "\xff"}), &options); SliceOptions options_step{-4, 4, 2}; this->CheckUnary( "binary_slice", - "[\"\", \"a\", \"ab\", \"Z\xc2\xa2\", \"aZ\xc2\xa2\", \"aP\xc2\xffZ\"]", - this->type(), "[\"\", \"a\", \"a\", \"Z\xa2\", \"a\xc2\", \"P\xff\"]", - &options_step); + this->MakeArray({"", "a", "ab", "Z\xc2\xa2", "aZ\xc2\xa2", "aP\xc2\xffZ"}), + this->MakeArray({"", "a", "a", "Z\xa2", "a\xc2", "P\xff"}), &options_step); SliceOptions options_step_neg{-1, 1, -2}; this->CheckUnary( "binary_slice", - "[\"\", \"a\", \"ab\", \"Z\xc2\xa2\", \"aZ\xc2\xa2\", \"aP\xc2\xffZ\"]", - this->type(), "[\"\", \"\", \"\", \"\xa2\", \"\xa2\", \"Z\xc2\"]", - &options_step_neg); + this->MakeArray({"", "a", "ab", "Z\xc2\xa2", "aZ\xc2\xa2", "aP\xc2\xffZ"}), + this->MakeArray({"", "", "", "\xa2", "\xa2", "Z\xc2"}), &options_step_neg); options_step_neg.stop = 0; this->CheckUnary( "binary_slice", - "[\"\", \"a\", \"ab\", \"Z\xc2\xa2\", \"aZ\xc2\xa2\", \"aP\xc2\xffZ\"]", - this->type(), "[\"\", \"\", \"b\", \"\xa2\", \"\xa2Z\", \"Z\xc2\"]", - &options_step_neg); + this->MakeArray({"", "a", "ab", "Z\xc2\xa2", "aZ\xc2\xa2", "aP\xc2\xffZ"}), + this->MakeArray({"", "", "b", "\xa2", "\xa2Z", "Z\xc2"}), &options_step_neg); } TYPED_TEST(TestStringKernels, PadAscii) { diff --git a/cpp/src/arrow/extension/json_test.cc b/cpp/src/arrow/extension/json_test.cc index b938ddb2cfef..ae0b34db384f 100644 --- a/cpp/src/arrow/extension/json_test.cc +++ b/cpp/src/arrow/extension/json_test.cc @@ -20,6 +20,7 @@ #include "arrow/array/validate.h" #include "arrow/ipc/test_common.h" #include "arrow/record_batch.h" +#include "arrow/testing/builder.h" #include "arrow/testing/gtest_util.h" #include "parquet/exception.h" @@ -64,7 +65,8 @@ TEST_F(TestJsonExtensionType, JsonRoundtrip) { TEST_F(TestJsonExtensionType, InvalidUTF8) { for (const auto& storage_type : {utf8(), large_utf8(), utf8_view()}) { auto json_type = json(storage_type); - auto invalid_input = ArrayFromJSON(storage_type, "[\"Ⱥa\xFFⱭ\", \"Ɽ\xe1\xbdⱤaA\"]"); + auto invalid_input = + BinaryArrayFromStrings(storage_type, {"Ⱥa\xFFⱭ", "Ɽ\xe1\xbdⱤaA"}); auto ext_arr = ExtensionType::WrapArray(json_type, invalid_input); ASSERT_RAISES_WITH_MESSAGE(Invalid, diff --git a/cpp/src/arrow/json/from_string.cc b/cpp/src/arrow/json/from_string.cc index e35a362f5a2e..9cb14a92a647 100644 --- a/cpp/src/arrow/json/from_string.cc +++ b/cpp/src/arrow/json/from_string.cc @@ -15,10 +15,16 @@ // specific language governing permissions and limitations // under the License. +#include +#include +#include #include +#include #include +#include #include #include +#include #include #include @@ -32,23 +38,21 @@ #include "arrow/array/builder_union.h" #include "arrow/chunked_array.h" #include "arrow/json/from_string.h" +#include "arrow/result.h" #include "arrow/scalar.h" +#include "arrow/status.h" +#include "arrow/type.h" #include "arrow/type_traits.h" #include "arrow/util/checked_cast.h" #include "arrow/util/decimal.h" #include "arrow/util/float16.h" #include "arrow/util/logging_internal.h" +#include "arrow/util/unreachable.h" #include "arrow/util/value_parsing.h" -#include "arrow/json/rapidjson_defs.h" +#include -#include -#include -#include -#include -#include - -namespace rj = arrow::rapidjson; +namespace sj = simdjson::ondemand; namespace arrow { @@ -62,32 +66,104 @@ using ::arrow::internal::checked_pointer_cast; namespace { -constexpr auto kParseFlags = rj::kParseFullPrecisionFlag | rj::kParseNanAndInfFlag; - -const char* JsonTypeName(rj::Type json_type) { - switch (json_type) { - case rapidjson::kNullType: - return "null"; - case rapidjson::kFalseType: - return "false"; - case rapidjson::kTrueType: - return "true"; - case rapidjson::kObjectType: - return "object"; - case rapidjson::kArrayType: +const char* JsonTypeName(sj::json_type type) { + switch (type) { + case sj::json_type::array: return "array"; - case rapidjson::kStringType: - return "string"; - case rapidjson::kNumberType: + case sj::json_type::object: + return "object"; + case sj::json_type::number: return "number"; + case sj::json_type::string: + return "string"; + case sj::json_type::boolean: + return "boolean"; + case sj::json_type::null: + return "null"; default: return "unknown"; } } -Status JSONTypeError(const char* expected_type, rj::Type json_type) { - return Status::Invalid("Expected ", expected_type, " or null, got JSON type ", - JsonTypeName(json_type)); +// Empty struct to represent the type of a simdjson null value +struct SimdjsonNull {}; + +template +struct JsonTypeNameOf; + +template <> +struct JsonTypeNameOf { + static constexpr const char* kValue = "array"; +}; +template <> +struct JsonTypeNameOf { + static constexpr const char* kValue = "object"; +}; +template <> +struct JsonTypeNameOf { + static constexpr const char* kValue = "string"; +}; +template <> +struct JsonTypeNameOf { + static constexpr const char* kValue = "boolean"; +}; +template <> +struct JsonTypeNameOf { + static constexpr const char* kValue = "null"; +}; +template <> +struct JsonTypeNameOf { + static constexpr const char* kValue = "number"; +}; +template <> +struct JsonTypeNameOf { + static constexpr const char* kValue = "number"; +}; +template <> +struct JsonTypeNameOf { + static constexpr const char* kValue = "number"; +}; + +template +constexpr const char* JsonTypeName() { + return JsonTypeNameOf::kValue; +} + +template +Result GetJsonAs(sj::value& value) { + SimdjsonValueType typed_value{}; + simdjson::error_code error_code; + if constexpr (std::is_same_v) { + // simdjson has no get<>() for null; probe it explicitly + bool is_null; + error_code = value.is_null().get(is_null); + if (error_code == simdjson::SUCCESS && !is_null) { + error_code = simdjson::INCORRECT_TYPE; + } + } else { + error_code = value.get(typed_value); + } + if (error_code != simdjson::SUCCESS) { + sj::json_type json_type; + if (value.type().get(json_type) != simdjson::SUCCESS) { + return Status::Invalid("Expected ", JsonTypeName(), + " or null, got malformed JSON value"); + } + return Status::Invalid("Expected ", JsonTypeName(), + " or null, got JSON type ", JsonTypeName(json_type)); + } + return typed_value; +} + +template +Result GetJsonResult( + simdjson::simdjson_result element, std::string_view error) { + SimdjsonValueType typed_value; + if (auto error_code = std::move(element).get(typed_value); + error_code != simdjson::SUCCESS) { + return Status::Invalid(error, simdjson::error_message(error_code)); + } + return typed_value; } class JSONConverter { @@ -96,11 +172,11 @@ class JSONConverter { virtual Status Init() { return Status::OK(); } - virtual Status AppendValue(const rj::Value& json_obj) = 0; + virtual Status AppendValue(sj::value& json_obj) = 0; Status AppendNull() { return this->builder()->AppendNull(); } - virtual Status AppendValues(const rj::Value& json_array) = 0; + virtual Result AppendValues(sj::array& json_array) = 0; virtual std::shared_ptr builder() = 0; @@ -124,20 +200,17 @@ Status GetConverter(const std::shared_ptr&, template class ConcreteConverter : public JSONConverter { public: - Result SizeOfJSONArray(const rj::Value& json_obj) { - if (!json_obj.IsArray()) { - return JSONTypeError("array", json_obj.GetType()); - } - return json_obj.Size(); - } - - Status AppendValues(const rj::Value& json_array) final { + Result AppendValues(sj::array& json_array) final { auto self = static_cast(this); - ARROW_ASSIGN_OR_RAISE(auto size, SizeOfJSONArray(json_array)); - for (uint32_t i = 0; i < size; ++i) { - RETURN_NOT_OK(self->AppendValue(json_array[i])); + int32_t num_elements = 0; + for (auto element : json_array) { + ARROW_ASSIGN_OR_RAISE(auto value, + GetJsonResult( + element, "Could not iterate elements of JSON array: ")); + RETURN_NOT_OK(self->AppendValue(value)); + num_elements++; } - return Status::OK(); + return num_elements; } const std::shared_ptr& value_type() { @@ -167,11 +240,9 @@ class NullConverter final : public ConcreteConverter { builder_ = std::make_shared(); } - Status AppendValue(const rj::Value& json_obj) override { - if (json_obj.IsNull()) { - return AppendNull(); - } - return JSONTypeError("null", json_obj.GetType()); + Status AppendValue(sj::value& json_obj) override { + ARROW_RETURN_NOT_OK(GetJsonAs(json_obj)); + return AppendNull(); } std::shared_ptr builder() override { return builder_; } @@ -190,17 +261,16 @@ class BooleanConverter final : public ConcreteConverter { builder_ = std::make_shared(); } - Status AppendValue(const rj::Value& json_obj) override { - if (json_obj.IsNull()) { + Status AppendValue(sj::value& json_obj) override { + if (json_obj.is_null()) { return AppendNull(); } - if (json_obj.IsBool()) { - return builder_->Append(json_obj.GetBool()); + int64_t int_value; + if (json_obj.get(int_value) == simdjson::SUCCESS) { + return builder_->Append(int_value != 0); } - if (json_obj.IsInt()) { - return builder_->Append(json_obj.GetInt() != 0); - } - return JSONTypeError("boolean", json_obj.GetType()); + ARROW_ASSIGN_OR_RAISE(bool bool_value, GetJsonAs(json_obj)); + return builder_->Append(bool_value); } std::shared_ptr builder() override { return builder_; } @@ -214,78 +284,119 @@ class BooleanConverter final : public ConcreteConverter { // Convert single signed integer value (also {Date,Time}{32,64} and Timestamp) template -enable_if_physical_signed_integer ConvertNumber(const rj::Value& json_obj, +enable_if_physical_signed_integer ConvertNumber(sj::value& json_obj, const DataType& type, typename T::c_type* out) { - if (json_obj.IsInt64()) { - int64_t v64 = json_obj.GetInt64(); - *out = static_cast(v64); - if (*out == v64) { - return Status::OK(); - } else { - return Status::Invalid("Value ", v64, " out of bounds for ", type); - } + *out = static_cast(0); + ARROW_ASSIGN_OR_RAISE(int64_t v64, GetJsonAs(json_obj)); + *out = static_cast(v64); + if (*out == v64) { + return Status::OK(); } else { - *out = static_cast(0); - return JSONTypeError("signed int", json_obj.GetType()); + return Status::Invalid("Value ", v64, " out of bounds for ", type); } } // Convert single unsigned integer value template -enable_if_unsigned_integer ConvertNumber(const rj::Value& json_obj, +enable_if_unsigned_integer ConvertNumber(sj::value& json_obj, const DataType& type, typename T::c_type* out) { - if (json_obj.IsUint64()) { - uint64_t v64 = json_obj.GetUint64(); - *out = static_cast(v64); - if (*out == v64) { - return Status::OK(); - } else { - return Status::Invalid("Value ", v64, " out of bounds for ", type); - } + *out = static_cast(0); + ARROW_ASSIGN_OR_RAISE(uint64_t v64, GetJsonAs(json_obj)); + *out = static_cast(v64); + if (*out == v64) { + return Status::OK(); } else { - *out = static_cast(0); - return JSONTypeError("unsigned int", json_obj.GetType()); + return Status::Invalid("Value ", v64, " out of bounds for ", type); + } +} + +// Match the std::string_view against NaN, Inf, Infinity with optional leading minus +std::optional NonFiniteDoubleFromString(std::string_view str) { + if (str == "NaN") { + return std::numeric_limits::quiet_NaN(); + } else if (str == "-NaN") { + return -std::numeric_limits::quiet_NaN(); + } else if (str == "Inf" || str == "Infinity") { + return std::numeric_limits::infinity(); + } else if (str == "-Inf" || str == "-Infinity") { + return -std::numeric_limits::infinity(); + } else { + return std::nullopt; + } +} + +std::optional NonFiniteDoubleFromRawToken(sj::value& json_obj) { + std::string_view token = json_obj.raw_json_token(); + // The raw token includes any trailing whitespace up to the next token + while (!token.empty() && std::isspace(static_cast(token.back()))) { + token.remove_suffix(1); } + return NonFiniteDoubleFromString(token); } // Convert float16/HalfFloatType template -enable_if_half_float ConvertNumber(const rj::Value& json_obj, - const DataType& type, uint16_t* out) { - if (json_obj.IsDouble()) { - double f64 = json_obj.GetDouble(); - *out = Float16(f64).bits(); - return Status::OK(); - } else if (json_obj.IsUint()) { - uint32_t u32t = json_obj.GetUint(); - double f64 = static_cast(u32t); - *out = Float16(f64).bits(); - return Status::OK(); - } else if (json_obj.IsInt()) { - int32_t i32t = json_obj.GetInt(); - double f64 = static_cast(i32t); - *out = Float16(f64).bits(); +enable_if_half_float ConvertNumber(sj::value& json_obj, const DataType& type, + uint16_t* out) { + *out = static_cast(0); + if (auto f64 = NonFiniteDoubleFromRawToken(json_obj); f64.has_value()) { + *out = Float16(f64.value()).bits(); return Status::OK(); - } else { - *out = static_cast(0); - return JSONTypeError("unsigned int", json_obj.GetType()); } + ARROW_ASSIGN_OR_RAISE(auto f64, GetJsonAs(json_obj)); + *out = Float16(f64).bits(); + return arrow::Status::OK(); } // Convert single floating point value template -enable_if_physical_floating_point ConvertNumber(const rj::Value& json_obj, +enable_if_physical_floating_point ConvertNumber(sj::value& json_obj, const DataType& type, typename T::c_type* out) { - if (json_obj.IsNumber()) { - *out = static_cast(json_obj.GetDouble()); + *out = static_cast(0); + if (auto f64 = NonFiniteDoubleFromRawToken(json_obj); f64.has_value()) { + *out = static_cast(f64.value()); return Status::OK(); - } else { - *out = static_cast(0); - return JSONTypeError("number", json_obj.GetType()); } + ARROW_ASSIGN_OR_RAISE(auto f64, GetJsonAs(json_obj)); + *out = static_cast(f64); + return arrow::Status::OK(); +} + +// ------------------------------------------------------------------------ +// Helper to process a JSON array with exactly kExpectedSize elements, calling +// one handler per element, in order. +using JsonElementHandler = std::function; + +template +Status ProcessJsonArrayElements( + sj::array& json_array, const char* error_context, + const std::array& handlers) { + auto it = json_array.begin(); + auto end = json_array.end(); + + size_t index = 0; + for (const auto& handler : handlers) { + if (it == end) { + return Status::Invalid(error_context, " must have exactly ", kExpectedSize, + " elements, had ", index); + } + + ARROW_ASSIGN_OR_RAISE( + sj::value element, + GetJsonResult(*it, "Could not iterate elements of JSON array: ")); + RETURN_NOT_OK(handler(element)); + ++it; + ++index; + } + + if (it != end) { + return Status::Invalid(error_context, " must have exactly ", kExpectedSize, + " elements, had more"); + } + return Status::OK(); } // ------------------------------------------------------------------------ @@ -303,8 +414,8 @@ class IntegerConverter final Status Init() override { return this->MakeConcreteBuilder(&builder_); } - Status AppendValue(const rj::Value& json_obj) override { - if (json_obj.IsNull()) { + Status AppendValue(sj::value& json_obj) override { + if (json_obj.is_null()) { return this->AppendNull(); } c_type value; @@ -330,8 +441,8 @@ class FloatConverter final : public ConcreteConverterMakeConcreteBuilder(&builder_); } - Status AppendValue(const rj::Value& json_obj) override { - if (json_obj.IsNull()) { + Status AppendValue(sj::value& json_obj) override { + if (json_obj.is_null()) { return this->AppendNull(); } c_type value; @@ -360,22 +471,19 @@ class DecimalConverter final Status Init() override { return this->MakeConcreteBuilder(&builder_); } - Status AppendValue(const rj::Value& json_obj) override { - if (json_obj.IsNull()) { + Status AppendValue(sj::value& json_obj) override { + if (json_obj.is_null()) { return this->AppendNull(); } - if (json_obj.IsString()) { - int32_t precision, scale; - DecimalValue d; - auto view = std::string_view(json_obj.GetString(), json_obj.GetStringLength()); - RETURN_NOT_OK(DecimalValue::FromString(view, &d, &precision, &scale)); - if (scale != decimal_type_->scale()) { - return Status::Invalid("Invalid scale for decimal: expected ", - decimal_type_->scale(), ", got ", scale); - } - return builder_->Append(d); + ARROW_ASSIGN_OR_RAISE(auto string_value, GetJsonAs(json_obj)); + int32_t precision, scale; + DecimalValue d; + RETURN_NOT_OK(DecimalValue::FromString(string_value, &d, &precision, &scale)); + if (scale != decimal_type_->scale()) { + return Status::Invalid("Invalid scale for decimal: expected ", + decimal_type_->scale(), ", got ", scale); } - return JSONTypeError("decimal string", json_obj.GetType()); + return builder_->Append(d); } std::shared_ptr builder() override { return builder_; } @@ -405,20 +513,18 @@ class TimestampConverter final : public ConcreteConverter { builder_ = std::make_shared(type, default_memory_pool()); } - Status AppendValue(const rj::Value& json_obj) override { - if (json_obj.IsNull()) { + Status AppendValue(sj::value& json_obj) override { + if (json_obj.is_null()) { return this->AppendNull(); } int64_t value; - if (json_obj.IsNumber()) { - RETURN_NOT_OK(ConvertNumber(json_obj, *this->type_, &value)); - } else if (json_obj.IsString()) { - std::string_view view(json_obj.GetString(), json_obj.GetStringLength()); + std::string_view view; + if (json_obj.get(view) == simdjson::SUCCESS) { if (!ParseValue(*timestamp_type_, view.data(), view.size(), &value)) { return Status::Invalid("couldn't parse timestamp from ", view); } } else { - return JSONTypeError("timestamp", json_obj.GetType()); + RETURN_NOT_OK(ConvertNumber(json_obj, *this->type_, &value)); } return builder_->Append(value); } @@ -441,21 +547,22 @@ class DayTimeIntervalConverter final builder_ = std::make_shared(default_memory_pool()); } - Status AppendValue(const rj::Value& json_obj) override { - if (json_obj.IsNull()) { + Status AppendValue(sj::value& json_obj) override { + if (json_obj.is_null()) { return this->AppendNull(); } + + ARROW_ASSIGN_OR_RAISE(auto array, GetJsonAs(json_obj)); + DayTimeIntervalType::DayMilliseconds value; - if (!json_obj.IsArray()) { - return JSONTypeError("array", json_obj.GetType()); - } - if (json_obj.Size() != 2) { - return Status::Invalid( - "day time interval pair must have exactly two elements, had ", json_obj.Size()); - } - RETURN_NOT_OK(ConvertNumber(json_obj[0], *this->type_, &value.days)); - RETURN_NOT_OK( - ConvertNumber(json_obj[1], *this->type_, &value.milliseconds)); + RETURN_NOT_OK(ProcessJsonArrayElements<2>( + array, "day-time interval", + {[this, &value](sj::value& elem) { + return ConvertNumber(elem, *this->type_, &value.days); + }, + [this, &value](sj::value& elem) { + return ConvertNumber(elem, *this->type_, &value.milliseconds); + }})); return builder_->Append(value); } @@ -473,23 +580,25 @@ class MonthDayNanoIntervalConverter final builder_ = std::make_shared(default_memory_pool()); } - Status AppendValue(const rj::Value& json_obj) override { - if (json_obj.IsNull()) { + Status AppendValue(sj::value& json_obj) override { + if (json_obj.is_null()) { return this->AppendNull(); } - MonthDayNanoIntervalType::MonthDayNanos value; - if (!json_obj.IsArray()) { - return JSONTypeError("array", json_obj.GetType()); - } - if (json_obj.Size() != 3) { - return Status::Invalid( - "month_day_nano_interval must have exactly 3 elements, had ", json_obj.Size()); - } - RETURN_NOT_OK(ConvertNumber(json_obj[0], *this->type_, &value.months)); - RETURN_NOT_OK(ConvertNumber(json_obj[1], *this->type_, &value.days)); - RETURN_NOT_OK( - ConvertNumber(json_obj[2], *this->type_, &value.nanoseconds)); + ARROW_ASSIGN_OR_RAISE(auto array, GetJsonAs(json_obj)); + + MonthDayNanoIntervalType::MonthDayNanos value; + RETURN_NOT_OK(ProcessJsonArrayElements<3>( + array, "month-day-nano interval", + {[this, &value](sj::value& elem) { + return ConvertNumber(elem, *this->type_, &value.months); + }, + [this, &value](sj::value& elem) { + return ConvertNumber(elem, *this->type_, &value.days); + }, + [this, &value](sj::value& elem) { + return ConvertNumber(elem, *this->type_, &value.nanoseconds); + }})); return builder_->Append(value); } @@ -510,16 +619,13 @@ class StringConverter final Status Init() override { return this->MakeConcreteBuilder(&builder_); } - Status AppendValue(const rj::Value& json_obj) override { - if (json_obj.IsNull()) { + Status AppendValue(sj::value& json_obj) override { + if (json_obj.is_null()) { return this->AppendNull(); } - if (json_obj.IsString()) { - auto view = std::string_view(json_obj.GetString(), json_obj.GetStringLength()); - return builder_->Append(view); - } else { - return JSONTypeError("string", json_obj.GetType()); - } + + ARROW_ASSIGN_OR_RAISE(auto view, GetJsonAs(json_obj)); + return builder_->Append(view); } std::shared_ptr builder() override { return builder_; } @@ -541,22 +647,18 @@ class FixedSizeBinaryConverter final Status Init() override { return this->MakeConcreteBuilder(&builder_); } - Status AppendValue(const rj::Value& json_obj) override { - if (json_obj.IsNull()) { + Status AppendValue(sj::value& json_obj) override { + if (json_obj.is_null()) { return this->AppendNull(); } - if (json_obj.IsString()) { - auto view = std::string_view(json_obj.GetString(), json_obj.GetStringLength()); - if (view.length() != static_cast(builder_->byte_width())) { - std::stringstream ss; - ss << "Invalid string length " << view.length() << " in JSON input for " - << this->type_->ToString(); - return Status::Invalid(ss.str()); - } - return builder_->Append(view); - } else { - return JSONTypeError("string", json_obj.GetType()); + ARROW_ASSIGN_OR_RAISE(auto view, GetJsonAs(json_obj)); + if (view.length() != static_cast(builder_->byte_width())) { + std::stringstream ss; + ss << "Invalid string length " << view.length() << " in JSON input for " + << this->type_->ToString(); + return Status::Invalid(ss.str()); } + return builder_->Append(view); } std::shared_ptr builder() override { return builder_; } @@ -588,14 +690,18 @@ class VarLengthListLikeConverter final return Status::OK(); } - Status AppendValue(const rj::Value& json_obj) override { - if (json_obj.IsNull()) { + Status AppendValue(sj::value& json_obj) override { + if (json_obj.is_null()) { return this->AppendNull(); } + ARROW_ASSIGN_OR_RAISE(auto array, GetJsonAs(json_obj)); + size_t num_elements; + if (array.count_elements().get(num_elements) != simdjson::SUCCESS) { + return Status::Invalid("Malformed JSON array for type ", this->type_->ToString()); + } // Extend the child converter with this JSON array - ARROW_ASSIGN_OR_RAISE(auto size, this->SizeOfJSONArray(json_obj)); - RETURN_NOT_OK(builder_->Append(true, size)); - return child_converter_->AppendValues(json_obj); + ARROW_RETURN_NOT_OK(builder_->Append(true, static_cast(num_elements))); + return child_converter_->AppendValues(array).status(); } std::shared_ptr builder() override { return builder_; } @@ -623,29 +729,29 @@ class MapConverter final : public ConcreteConverter { return Status::OK(); } - Status AppendValue(const rj::Value& json_obj) override { - if (json_obj.IsNull()) { + Status AppendValue(sj::value& json_obj) override { + if (json_obj.is_null()) { return this->AppendNull(); } RETURN_NOT_OK(builder_->Append()); - if (!json_obj.IsArray()) { - return JSONTypeError("array", json_obj.GetType()); - } - auto size = json_obj.Size(); - for (uint32_t i = 0; i < size; ++i) { - const auto& json_pair = json_obj[i]; - if (!json_pair.IsArray()) { - return JSONTypeError("array", json_pair.GetType()); - } - if (json_pair.Size() != 2) { - return Status::Invalid("key item pair must have exactly two elements, had ", - json_pair.Size()); - } - if (json_pair[0].IsNull()) { - return Status::Invalid("null key is invalid"); - } - RETURN_NOT_OK(key_converter_->AppendValue(json_pair[0])); - RETURN_NOT_OK(item_converter_->AppendValue(json_pair[1])); + ARROW_ASSIGN_OR_RAISE(auto array, GetJsonAs(json_obj)); + + for (auto json_pair_result : array) { + ARROW_ASSIGN_OR_RAISE( + auto json_pair, + GetJsonResult(json_pair_result, + "Could not iterate elements of JSON array: ")); + ARROW_ASSIGN_OR_RAISE(auto json_pair_array, GetJsonAs(json_pair)); + + RETURN_NOT_OK(ProcessJsonArrayElements<2>( + json_pair_array, "key-item pair", + {[this](sj::value& key) { + if (key.is_null()) { + return Status::Invalid("null key is invalid"); + } + return key_converter_->AppendValue(key); + }, + [this](sj::value& item) { return item_converter_->AppendValue(item); }})); } return Status::OK(); } @@ -674,15 +780,16 @@ class FixedSizeListConverter final : public ConcreteConverterAppendNull(); } RETURN_NOT_OK(builder_->Append()); // Extend the child converter with this JSON array - RETURN_NOT_OK(child_converter_->AppendValues(json_obj)); - if (json_obj.GetArray().Size() != static_cast(list_size_)) { - return Status::Invalid("incorrect list size ", json_obj.GetArray().Size()); + ARROW_ASSIGN_OR_RAISE(auto array, GetJsonAs(json_obj)); + ARROW_ASSIGN_OR_RAISE(int32_t size, child_converter_->AppendValues(array)); + if (size != list_size_) { + return Status::Invalid("incorrect list size ", size); } return Status::OK(); } @@ -710,6 +817,9 @@ class StructConverter final : public ConcreteConverter { child_converters_.push_back(child_converter); child_builders.push_back(child_converter->builder()); } + for (int32_t i = 0; i < type_->num_fields(); ++i) { + field_index_.emplace(type_->field(i)->name(), i); + } builder_ = std::make_shared(type_, default_memory_pool(), std::move(child_builders)); return Status::OK(); @@ -718,45 +828,68 @@ class StructConverter final : public ConcreteConverter { // Append a JSON value that is either an array of N elements in order // or an object mapping struct names to values (omitted struct members // are mapped to null). - Status AppendValue(const rj::Value& json_obj) override { - if (json_obj.IsNull()) { + Status AppendValue(sj::value& json_obj) override { + if (json_obj.is_null()) { return this->AppendNull(); } - if (json_obj.IsArray()) { - auto size = json_obj.Size(); - auto expected_size = static_cast(type_->num_fields()); + sj::array array; + if (json_obj.get(array) == simdjson::SUCCESS) { + auto expected_size = static_cast(type_->num_fields()); + size_t size; + if (array.count_elements().get(size) != simdjson::SUCCESS) { + return Status::Invalid("Malformed JSON array for type ", type_->ToString()); + } if (size != expected_size) { return Status::Invalid("Expected array of size ", expected_size, ", got array of size ", size); } - for (uint32_t i = 0; i < size; ++i) { - RETURN_NOT_OK(child_converters_[i]->AppendValue(json_obj[i])); + size_t i = 0; + for (auto child : array) { + ARROW_ASSIGN_OR_RAISE(auto child_value, + GetJsonResult( + child, "Could not iterate elements of JSON array: ")); + RETURN_NOT_OK(child_converters_[i]->AppendValue(child_value)); + ++i; } return builder_->Append(); } - if (json_obj.IsObject()) { - auto remaining = json_obj.MemberCount(); - auto num_children = type_->num_fields(); - for (int32_t i = 0; i < num_children; ++i) { - const auto& field = type_->field(i); - auto it = json_obj.FindMember(field->name()); - if (it != json_obj.MemberEnd()) { - --remaining; - RETURN_NOT_OK(child_converters_[i]->AppendValue(it->value)); - } else { - RETURN_NOT_OK(child_converters_[i]->AppendNull()); - } + ARROW_ASSIGN_OR_RAISE(auto object, GetJsonAs(json_obj)); + // Iterate the object fields in JSON order (the on-demand API is + // forward-only, so per-field lookups would be quadratic and would also + // compare against raw, still-escaped keys). Fields absent from the JSON + // are appended as null afterwards. + auto num_fields = type_->num_fields(); + std::vector field_seen(num_fields, false); + for (auto field_result : object) { + ARROW_ASSIGN_OR_RAISE( + auto field, + GetJsonResult(field_result, "Error getting field of object: ")); + std::string_view key; + if (field.unescaped_key(/*allow_replacement=*/false).get(key) != + simdjson::SUCCESS) { + return Status::Invalid("Malformed key in JSON object for type ", + type_->ToString()); } - if (remaining > 0) { - rj::StringBuffer sb; - rj::Writer writer(sb); - json_obj.Accept(writer); - return Status::Invalid("Unexpected members in JSON object for type ", - type_->ToString(), " Object: ", sb.GetString()); + auto it = field_index_.find(key); + if (it == field_index_.end()) { + return Status::Invalid("Unexpected member \"", key, "\" in JSON object for type ", + type_->ToString()); + } + const int32_t field_num = it->second; + if (field_seen[field_num]) { + return Status::Invalid("Duplicate member \"", key, "\" in JSON object for type ", + type_->ToString()); + } + field_seen[field_num] = true; + sj::value value = field.value(); + RETURN_NOT_OK(child_converters_[field_num]->AppendValue(value)); + } + for (int32_t i = 0; i < num_fields; ++i) { + if (!field_seen[i]) { + RETURN_NOT_OK(child_converters_[i]->AppendNull()); } - return builder_->Append(); } - return JSONTypeError("array or object", json_obj.GetType()); + return builder_->Append(); } std::shared_ptr builder() override { return builder_; } @@ -764,6 +897,8 @@ class StructConverter final : public ConcreteConverter { private: std::shared_ptr builder_; std::vector> child_converters_; + // keys are views into the field names owned by type_ + std::unordered_map field_index_; }; // ------------------------------------------------------------------------ @@ -801,40 +936,43 @@ class UnionConverter final : public ConcreteConverter { // Append a JSON value that must be a 2-long array, containing the type_id // and value of the UnionArray's slot. - Status AppendValue(const rj::Value& json_obj) override { - if (json_obj.IsNull()) { + Status AppendValue(sj::value& json_obj) override { + if (json_obj.is_null()) { return this->AppendNull(); } - if (!json_obj.IsArray()) { - return JSONTypeError("array", json_obj.GetType()); - } - if (json_obj.Size() != 2) { - return Status::Invalid("Expected [type_id, value] pair, got array of size ", - json_obj.Size()); - } - const auto& id_obj = json_obj[0]; - if (!id_obj.IsInt()) { - return JSONTypeError("int", id_obj.GetType()); - } - - auto id = static_cast(id_obj.GetInt()); - auto child_num = type_id_to_child_num_[id]; - if (child_num == -1) { - return Status::Invalid("type_id ", id, " not found in ", *type_); - } - auto child_converter = child_converters_[child_num]; - if (mode_ == UnionMode::SPARSE) { - RETURN_NOT_OK(checked_cast(*builder_).Append(id)); - for (auto&& other_converter : child_converters_) { - if (other_converter != child_converter) { - RETURN_NOT_OK(other_converter->AppendNull()); - } - } - } else { - RETURN_NOT_OK(checked_cast(*builder_).Append(id)); - } - return child_converter->AppendValue(json_obj[1]); + ARROW_ASSIGN_OR_RAISE(auto array, GetJsonAs(json_obj)); + + int8_t id = 0; + std::shared_ptr child_converter; + + RETURN_NOT_OK(ProcessJsonArrayElements<2>( + array, "[type_id, value] pair", + {[this, &id, &child_converter](sj::value& id_elem) { + ARROW_ASSIGN_OR_RAISE(auto id_value, GetJsonAs(id_elem)); + id = static_cast(id_value); + auto child_num = type_id_to_child_num_[id]; + if (child_num == -1) { + return Status::Invalid("type_id ", id, " not found in ", *type_); + } + child_converter = child_converters_[child_num]; + + if (mode_ == UnionMode::SPARSE) { + RETURN_NOT_OK(checked_cast(*builder_).Append(id)); + for (auto&& other_converter : child_converters_) { + if (other_converter != child_converter) { + RETURN_NOT_OK(other_converter->AppendNull()); + } + } + } else { + RETURN_NOT_OK(checked_cast(*builder_).Append(id)); + } + return Status::OK(); + }, + [&child_converter](sj::value& value_elem) { + return child_converter->AppendValue(value_elem); + }})); + return Status::OK(); } std::shared_ptr builder() override { return builder_; } @@ -980,15 +1118,27 @@ Result> ArrayFromJSONString(const std::shared_ptr converter; RETURN_NOT_OK(GetConverter(type, &converter)); - rj::Document json_doc; - json_doc.Parse(json_string.data(), json_string.length()); - if (json_doc.HasParseError()) { - return Status::Invalid("JSON parse error at offset ", json_doc.GetErrorOffset(), ": ", - GetParseError_En(json_doc.GetParseError())); + simdjson::padded_string padded_string{json_string}; + + sj::parser parser; + sj::document json_doc; + auto error = parser.iterate(padded_string).get(json_doc); + if (error) { + return Status::Invalid("JSON parse error: ", simdjson::error_message(error)); + } + sj::value json_obj; + if (auto error_code = json_doc.get_value().get(json_obj); + error_code != simdjson::SUCCESS) { + return Status::Invalid("JSON parse error: ", simdjson::error_message(error_code)); } + ARROW_ASSIGN_OR_RAISE(auto array, GetJsonAs(json_obj)); // The JSON document should be an array, append it - RETURN_NOT_OK(converter->AppendValues(json_doc)); + RETURN_NOT_OK(converter->AppendValues(array)); + // The parser is lazy and does not look past the end of the array by itself + if (!json_doc.at_end()) { + return Status::Invalid("JSON parse error: trailing content after JSON array"); + } std::shared_ptr out; RETURN_NOT_OK(converter->Finish(&out)); return out; @@ -1036,17 +1186,36 @@ Result> ScalarFromJSONString( std::shared_ptr converter; RETURN_NOT_OK(GetConverter(type, &converter)); - rj::Document json_doc; - json_doc.Parse(json_string.data(), json_string.length()); - if (json_doc.HasParseError()) { - return Status::Invalid("JSON parse error at offset ", json_doc.GetErrorOffset(), ": ", - GetParseError_En(json_doc.GetParseError())); + simdjson::padded_string padded_string{ + arrow::internal::JoinToString("[", json_string, "]")}; + + sj::parser parser; + sj::document json_doc; + auto error = parser.iterate(padded_string).get(json_doc); + if (error) { + return Status::Invalid("JSON parse error: ", simdjson::error_message(error)); + } + + sj::value json_obj; + if (auto error_code = json_doc.get_value().get(json_obj); + error_code != simdjson::SUCCESS) { + return Status::Invalid("JSON parse error: ", simdjson::error_message(error_code)); + } + ARROW_ASSIGN_OR_RAISE(auto singleton_array, GetJsonAs(json_obj)); + + ARROW_ASSIGN_OR_RAISE(int32_t num_elements, converter->AppendValues(singleton_array)); + if (num_elements != 1) { + return Status::Invalid("Expected exactly one JSON value, got ", num_elements, + " values"); + } + // The parser is lazy and does not look past the end of the wrapped value by + // itself, e.g. the input "1] , [2" would otherwise be accepted as 1 + if (!json_doc.at_end()) { + return Status::Invalid("JSON parse error: trailing content after JSON value"); } std::shared_ptr array; - RETURN_NOT_OK(converter->AppendValue(json_doc)); RETURN_NOT_OK(converter->Finish(&array)); - DCHECK_EQ(array->length(), 1); return array->GetScalar(0); } diff --git a/cpp/src/arrow/json/from_string_test.cc b/cpp/src/arrow/json/from_string_test.cc index 654450462e36..817cfe52ee72 100644 --- a/cpp/src/arrow/json/from_string_test.cc +++ b/cpp/src/arrow/json/from_string_test.cc @@ -217,6 +217,9 @@ TYPED_TEST_P(TestIntegersFromString, Errors) { ASSERT_RAISES(Invalid, ArrayFromJSONString(type, "[0.0]")); ASSERT_RAISES(Invalid, ArrayFromJSONString(type, "[\"0\"]")); ASSERT_RAISES(Invalid, ArrayFromJSONString(type, "[[0]]")); + ASSERT_RAISES(Invalid, ArrayFromJSONString(type, "[0]]")); + ASSERT_RAISES(Invalid, ArrayFromJSONString(type, "[0], [1]")); + ASSERT_RAISES(Invalid, ArrayFromJSONString(type, "[0] junk")); } TYPED_TEST_P(TestIntegersFromString, OutOfBounds) { @@ -305,9 +308,9 @@ TYPED_TEST_P(TestStringsFromString, Basics) { AssertJSONArray(type, "[\"\xc3\xa9\"]", {"\xc3\xa9"}); if (!T::is_utf8) { - // Arbitrary binary (non-UTF8) sequence in string + // Arbitrary binary (non-UTF8) sequences cannot be represented s = "\xff\x9f"; - AssertJSONArray(type, "[\"" + s + "\"]", {s}); + ASSERT_RAISES(Invalid, ArrayFromJSONString(type, "[\"" + s + "\"]")); } // Bytes < 0x20 can be represented as JSON unicode escapes @@ -441,7 +444,8 @@ TEST(TestDoubleFromString, Basics) { AssertJSONArray(type, "[1, 2.5, -3e4]", {1.0, 2.5, -3.0e4}); AssertJSONArray(type, "[-0.0, Inf, -Inf, null]", {true, true, true, false}, {-0.0, INFINITY, -INFINITY, 0.0}); - + AssertJSONArray(type, "[Infinity, -Infinity, null]", {true, true, false}, + {INFINITY, -INFINITY, 0.0}); ASSERT_OK_AND_ASSIGN(actual, ArrayFromJSONString(type, "[NaN]")); ASSERT_OK(actual->ValidateFull()); double value = checked_cast(*actual).Value(0); @@ -536,9 +540,10 @@ TEST(TestFixedSizeBinaryFromString, Basics) { {"foo", "bar"}); AssertJSONArray(type, "[null, \"foo\"]", {false, true}, {"", "foo"}); - // Arbitrary binary (non-UTF8) sequence in string + // Arbitrary binary (non-UTF8) sequences cannot be represented: the JSON + // parser requires its input to be valid UTF-8 std::string s = "\xff\x9f\xcc"; - AssertJSONArray(type, "[\"" + s + "\"]", {s}); + ASSERT_RAISES(Invalid, ArrayFromJSONString(type, "[\"" + s + "\"]")); } TEST(TestFixedSizeBinaryFromString, Errors) { @@ -1563,6 +1568,9 @@ TEST(TestScalarFromJSONString, Errors) { ASSERT_RAISES(Invalid, ScalarFromJSONString(binary(), "[]")); ASSERT_RAISES(Invalid, ScalarFromJSONString(boolean(), "0.0")); ASSERT_RAISES(Invalid, ScalarFromJSONString(boolean(), "\"true\"")); + ASSERT_RAISES(Invalid, ScalarFromJSONString(int64(), "1, 2")); + ASSERT_RAISES(Invalid, ScalarFromJSONString(int64(), "1] , [2")); + ASSERT_RAISES(Invalid, ScalarFromJSONString(int64(), "1 junk")); } TEST(TestDictScalarFromJSONString, Basics) { diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index 4096b8cd1788..c9e49ab66abb 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -36,6 +36,7 @@ #include "arrow/memory_pool.h" #include "arrow/scalar.h" #include "arrow/status.h" +#include "arrow/testing/builder.h" #include "arrow/testing/extension_type.h" #include "arrow/testing/gtest_util.h" #include "arrow/testing/random.h" @@ -1395,8 +1396,10 @@ class TestListLikeScalar : public ::testing::Test { } { - // Invalid UTF8 in child data - ScalarType scalar(ArrayFromJSON(utf8(), "[null, null, \"\xff\"]")); + std::shared_ptr invalid_utf8; + ArrayFromVector({false, false, true}, {"", "", "\xff"}, + &invalid_utf8); + ScalarType scalar(invalid_utf8); ASSERT_OK(scalar.Validate()); ASSERT_RAISES(Invalid, scalar.ValidateFull()); } diff --git a/cpp/src/arrow/testing/builder.h b/cpp/src/arrow/testing/builder.h index 6beb7760e3bb..2d3cbbe9a458 100644 --- a/cpp/src/arrow/testing/builder.h +++ b/cpp/src/arrow/testing/builder.h @@ -92,6 +92,40 @@ void ArrayFromVector(const std::vector& values, std::shared_ptr* ArrayFromVector(type, values, out); } +// BinaryArrayFromStrings: construct an Array of any binary-like type from +// string values, dispatching on the runtime type id +inline std::shared_ptr BinaryArrayFromStrings( + const std::shared_ptr& type, const std::vector& values) { + std::shared_ptr array; + switch (type->id()) { + case Type::BINARY: + ArrayFromVector(type, values, &array); + break; + case Type::STRING: + ArrayFromVector(type, values, &array); + break; + case Type::LARGE_BINARY: + ArrayFromVector(type, values, &array); + break; + case Type::LARGE_STRING: + ArrayFromVector(type, values, &array); + break; + case Type::BINARY_VIEW: + ArrayFromVector(type, values, &array); + break; + case Type::STRING_VIEW: + ArrayFromVector(type, values, &array); + break; + case Type::FIXED_SIZE_BINARY: + ArrayFromVector(type, values, &array); + break; + default: + ADD_FAILURE() << "unsupported type for binary test data: " << type->ToString(); + break; + } + return array; +} + // ChunkedArrayFromVector: construct a ChunkedArray from vectors of C values template diff --git a/cpp/src/parquet/arrow/arrow_reader_writer_test.cc b/cpp/src/parquet/arrow/arrow_reader_writer_test.cc index 8735aea731ce..78d441b619f6 100644 --- a/cpp/src/parquet/arrow/arrow_reader_writer_test.cc +++ b/cpp/src/parquet/arrow/arrow_reader_writer_test.cc @@ -1495,7 +1495,12 @@ class TestBinaryLikeParquetIO : public ParquetIOTestBase { const std::shared_ptr& fallback_type) { const auto specific_array = ::arrow::ArrayFromJSON(specific_type, json); const auto fallback_array = ::arrow::ArrayFromJSON(fallback_type, json); + CheckRoundTrip(specific_array, fallback_array, binary_type); + } + void CheckRoundTrip(const std::shared_ptr& specific_array, + const std::shared_ptr& fallback_array, + ::arrow::Type::type binary_type) { // When the original Arrow schema isn't stored, the array is decoded as // the fallback type (since there is no specific Parquet logical // type for it). @@ -1521,13 +1526,27 @@ class TestBinaryLikeParquetIO : public ParquetIOTestBase { }; TEST_F(TestBinaryLikeParquetIO, LargeBinary) { - CheckRoundTrip("[\"foo\", \"\", null, \"\xff\"]", ::arrow::Type::LARGE_BINARY, - ::arrow::large_binary(), ::arrow::binary()); + const std::vector is_valid = {true, true, false, true}; + const std::vector values = {"foo", "", "", "\xff"}; + std::shared_ptr specific_array; + ::arrow::ArrayFromVector<::arrow::LargeBinaryType, std::string>(is_valid, values, + &specific_array); + std::shared_ptr fallback_array; + ::arrow::ArrayFromVector<::arrow::BinaryType, std::string>(is_valid, values, + &fallback_array); + CheckRoundTrip(specific_array, fallback_array, ::arrow::Type::LARGE_BINARY); } TEST_F(TestBinaryLikeParquetIO, BinaryView) { - CheckRoundTrip("[\"foo\", \"\", null, \"\xff\"]", ::arrow::Type::BINARY_VIEW, - ::arrow::binary_view(), ::arrow::binary()); + const std::vector is_valid = {true, true, false, true}; + const std::vector values = {"foo", "", "", "\xff"}; + std::shared_ptr specific_array; + ::arrow::ArrayFromVector<::arrow::BinaryViewType, std::string>(is_valid, values, + &specific_array); + std::shared_ptr fallback_array; + ::arrow::ArrayFromVector<::arrow::BinaryType, std::string>(is_valid, values, + &fallback_array); + CheckRoundTrip(specific_array, fallback_array, ::arrow::Type::BINARY_VIEW); } TEST_F(TestBinaryLikeParquetIO, LargeString) { diff --git a/python/pyarrow/src/arrow/python/gdb.cc b/python/pyarrow/src/arrow/python/gdb.cc index 2a7d2eda4bf2..89135239f26b 100644 --- a/python/pyarrow/src/arrow/python/gdb.cc +++ b/python/pyarrow/src/arrow/python/gdb.cc @@ -17,9 +17,13 @@ #include #include +#include +#include #include +#include #include "arrow/array.h" +#include "arrow/array/builder_binary.h" #include "arrow/chunked_array.h" #include "arrow/datum.h" #include "arrow/extension/uuid.h" @@ -29,6 +33,7 @@ #include "arrow/scalar.h" #include "arrow/table.h" #include "arrow/type.h" +#include "arrow/util/checked_cast.h" #include "arrow/util/debug.h" #include "arrow/util/decimal.h" #include "arrow/util/key_value_metadata.h" @@ -43,6 +48,8 @@ using json::ArrayFromJSONString; using json::ChunkedArrayFromJSONString; using json::ScalarFromJSONString; +using ::arrow::internal::checked_cast; + namespace gdb { // Add a nested `arrow` namespace to exercise type lookup from GDB (ARROW-15652) @@ -69,6 +76,25 @@ std::shared_ptr SliceArrayFromJSON(const std::shared_ptr& ty, } } +template +std::shared_ptr BinaryArrayFromStrings( + const std::shared_ptr& type, + const std::vector>& values) { + std::unique_ptr builder; + ARROW_CHECK_OK(MakeBuilder(default_memory_pool(), type, &builder)); + auto& concrete_builder = checked_cast(*builder); + for (const auto& value : values) { + if (value.has_value()) { + ARROW_CHECK_OK(concrete_builder.Append(*value)); + } else { + ARROW_CHECK_OK(concrete_builder.AppendNull()); + } + } + std::shared_ptr array; + ARROW_CHECK_OK(concrete_builder.Finish(&array)); + return array; +} + } // namespace void TestSession() { @@ -448,18 +474,22 @@ void TestSession() { decimal256(50, 6), R"([null, "-123456789012345678901234567890123456789.012345"])"); auto heap_decimal128_array_sliced = heap_decimal128_array->Slice(1, 1); - auto heap_fixed_size_binary_array = - SliceArrayFromJSON(fixed_size_binary(3), "[null, \"abc\", \"\\u0000\\u001f\xff\"]"); + auto heap_fixed_size_binary_array = BinaryArrayFromStrings( + fixed_size_binary(3), {std::nullopt, "abc", std::string("\x00\x1f\xff", 3)}); auto heap_fixed_size_binary_array_zero_width = SliceArrayFromJSON(fixed_size_binary(0), R"([null, ""])"); auto heap_fixed_size_binary_array_sliced = heap_fixed_size_binary_array->Slice(1, 1); - const char* json_binary_array = "[null, \"abcd\", \"\\u0000\\u001f\xff\"]"; - auto heap_binary_array = SliceArrayFromJSON(binary(), json_binary_array); - auto heap_large_binary_array = SliceArrayFromJSON(large_binary(), json_binary_array); - const char* json_string_array = "[null, \"héhé\", \"invalid \xff char\"]"; - auto heap_string_array = SliceArrayFromJSON(utf8(), json_string_array); - auto heap_large_string_array = SliceArrayFromJSON(large_utf8(), json_string_array); + const std::vector> binary_values = { + std::nullopt, "abcd", std::string("\x00\x1f\xff", 3)}; + auto heap_binary_array = BinaryArrayFromStrings(binary(), binary_values); + auto heap_large_binary_array = + BinaryArrayFromStrings(large_binary(), binary_values); + const std::vector> string_values = {std::nullopt, "héhé", + "invalid \xff char"}; + auto heap_string_array = BinaryArrayFromStrings(utf8(), string_values); + auto heap_large_string_array = + BinaryArrayFromStrings(large_utf8(), string_values); auto heap_binary_array_sliced = heap_binary_array->Slice(1, 1); // ChunkedArray