pitrou commented on code in PR #50653:
URL: https://github.com/apache/arrow/pull/50653#discussion_r3663516406
##########
cpp/src/arrow/json/from_string.cc:
##########
@@ -214,78 +268,132 @@ class BooleanConverter final : public
ConcreteConverter<BooleanConverter> {
// Convert single signed integer value (also {Date,Time}{32,64} and Timestamp)
template <typename T>
-enable_if_physical_signed_integer<T, Status> ConvertNumber(const rj::Value&
json_obj,
+enable_if_physical_signed_integer<T, Status> 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<typename T::c_type>(v64);
- if (*out == v64) {
- return Status::OK();
- } else {
- return Status::Invalid("Value ", v64, " out of bounds for ", type);
- }
+ *out = static_cast<typename T::c_type>(0);
+ ARROW_ASSIGN_OR_RAISE(int64_t v64, GetAs<int64_t>(json_obj));
+ *out = static_cast<typename T::c_type>(v64);
+ if (*out == v64) {
+ return Status::OK();
} else {
- *out = static_cast<typename T::c_type>(0);
- return JSONTypeError("signed int", json_obj.GetType());
+ return Status::Invalid("Value ", v64, " out of bounds for ", type);
}
}
// Convert single unsigned integer value
template <typename T>
-enable_if_unsigned_integer<T, Status> ConvertNumber(const rj::Value& json_obj,
+enable_if_unsigned_integer<T, Status> 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<typename T::c_type>(v64);
- if (*out == v64) {
- return Status::OK();
- } else {
- return Status::Invalid("Value ", v64, " out of bounds for ", type);
- }
+ *out = static_cast<typename T::c_type>(0);
+ ARROW_ASSIGN_OR_RAISE(uint64_t v64, GetAs<uint64_t>(json_obj));
+ *out = static_cast<typename T::c_type>(v64);
+ if (*out == v64) {
+ return Status::OK();
} else {
- *out = static_cast<typename T::c_type>(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<double> NonFiniteDoubleFromString(std::string_view str) {
+ if (str == "NaN") {
+ return std::numeric_limits<double>::quiet_NaN();
+ } else if (str == "-NaN") {
+ return -std::numeric_limits<double>::quiet_NaN();
+ } else if (str == "Inf" || str == "Infinity") {
+ return std::numeric_limits<double>::infinity();
+ } else if (str == "-Inf" || str == "-Infinity") {
+ return -std::numeric_limits<double>::infinity();
+ } else {
+ return std::nullopt;
+ }
+}
+
+std::optional<double> 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<unsigned
char>(token.back()))) {
+ token.remove_suffix(1);
}
+ return NonFiniteDoubleFromString(token);
}
// Convert float16/HalfFloatType
template <typename T>
-enable_if_half_float<T, Status> 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<double>(u32t);
- *out = Float16(f64).bits();
+enable_if_half_float<T, Status> ConvertNumber(sj::value& json_obj, const
DataType& type,
+ uint16_t* out) {
+ *out = static_cast<uint16_t>(0);
+ if (auto f64 = NonFiniteDoubleFromRawToken(json_obj); f64.has_value()) {
+ *out = Float16(f64.value()).bits();
return Status::OK();
- } else if (json_obj.IsInt()) {
- int32_t i32t = json_obj.GetInt();
- double f64 = static_cast<double>(i32t);
- *out = Float16(f64).bits();
- return Status::OK();
- } else {
- *out = static_cast<uint16_t>(0);
- return JSONTypeError("unsigned int", json_obj.GetType());
}
+ ARROW_ASSIGN_OR_RAISE(auto f64, GetAs<double>(json_obj));
+ *out = Float16(f64).bits();
+ return arrow::Status::OK();
}
// Convert single floating point value
template <typename T>
-enable_if_physical_floating_point<T, Status> ConvertNumber(const rj::Value&
json_obj,
+enable_if_physical_floating_point<T, Status> ConvertNumber(sj::value& json_obj,
const DataType&
type,
typename T::c_type*
out) {
- if (json_obj.IsNumber()) {
- *out = static_cast<typename T::c_type>(json_obj.GetDouble());
+ *out = static_cast<typename T::c_type>(0);
+ if (auto f64 = NonFiniteDoubleFromRawToken(json_obj); f64.has_value()) {
+ *out = static_cast<typename T::c_type>(f64.value());
return Status::OK();
- } else {
- *out = static_cast<typename T::c_type>(0);
- return JSONTypeError("number", json_obj.GetType());
}
+ ARROW_ASSIGN_OR_RAISE(auto f64, GetAs<double>(json_obj));
+ *out = static_cast<typename T::c_type>(f64);
+ return arrow::Status::OK();
+}
+
+// ------------------------------------------------------------------------
+// Helper to process a JSON array with exactly N elements, calling a handler
for each.
+// Each handler is a callable taking sj::value& and returning Status.
+template <typename... Handlers>
+Status ProcessJsonArrayElements(sj::array& json_array, const char*
error_context,
+ Handlers&&... handlers) {
+ constexpr size_t expected_size = sizeof...(Handlers);
+ auto it = json_array.begin();
+ auto end = json_array.end();
+
+ size_t index = 0;
+ Status result = Status::OK();
+
+ auto process_one = [&](auto&& handler) -> bool {
+ if (!result.ok()) return false;
+
+ if (it == end) {
+ result = Status::Invalid(error_context, " must have exactly ",
expected_size,
+ " elements, had ", index);
+ return false;
+ }
+
+ sj::value element;
+ auto error = (*it).get(element);
+ if (error) {
+ result = Status::Invalid("Failed to get element ", index, " from ",
error_context);
+ return false;
+ }
+
+ result = handler(element);
+ ++it;
+ ++index;
+ return result.ok();
+ };
+
+ // Use fold expression to process all handlers in order
+ (process_one(std::forward<Handlers>(handlers)) && ...);
Review Comment:
Note that `Status` supports the `&` operator, which might allow simplifying
this a bit.
##########
cpp/src/arrow/json/from_string.cc:
##########
@@ -214,78 +268,132 @@ class BooleanConverter final : public
ConcreteConverter<BooleanConverter> {
// Convert single signed integer value (also {Date,Time}{32,64} and Timestamp)
template <typename T>
-enable_if_physical_signed_integer<T, Status> ConvertNumber(const rj::Value&
json_obj,
+enable_if_physical_signed_integer<T, Status> 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<typename T::c_type>(v64);
- if (*out == v64) {
- return Status::OK();
- } else {
- return Status::Invalid("Value ", v64, " out of bounds for ", type);
- }
+ *out = static_cast<typename T::c_type>(0);
+ ARROW_ASSIGN_OR_RAISE(int64_t v64, GetAs<int64_t>(json_obj));
+ *out = static_cast<typename T::c_type>(v64);
+ if (*out == v64) {
+ return Status::OK();
} else {
- *out = static_cast<typename T::c_type>(0);
- return JSONTypeError("signed int", json_obj.GetType());
+ return Status::Invalid("Value ", v64, " out of bounds for ", type);
}
}
// Convert single unsigned integer value
template <typename T>
-enable_if_unsigned_integer<T, Status> ConvertNumber(const rj::Value& json_obj,
+enable_if_unsigned_integer<T, Status> 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<typename T::c_type>(v64);
- if (*out == v64) {
- return Status::OK();
- } else {
- return Status::Invalid("Value ", v64, " out of bounds for ", type);
- }
+ *out = static_cast<typename T::c_type>(0);
+ ARROW_ASSIGN_OR_RAISE(uint64_t v64, GetAs<uint64_t>(json_obj));
+ *out = static_cast<typename T::c_type>(v64);
+ if (*out == v64) {
+ return Status::OK();
} else {
- *out = static_cast<typename T::c_type>(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<double> NonFiniteDoubleFromString(std::string_view str) {
+ if (str == "NaN") {
+ return std::numeric_limits<double>::quiet_NaN();
+ } else if (str == "-NaN") {
+ return -std::numeric_limits<double>::quiet_NaN();
+ } else if (str == "Inf" || str == "Infinity") {
+ return std::numeric_limits<double>::infinity();
+ } else if (str == "-Inf" || str == "-Infinity") {
+ return -std::numeric_limits<double>::infinity();
+ } else {
+ return std::nullopt;
+ }
+}
+
+std::optional<double> 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<unsigned
char>(token.back()))) {
+ token.remove_suffix(1);
}
+ return NonFiniteDoubleFromString(token);
}
// Convert float16/HalfFloatType
template <typename T>
-enable_if_half_float<T, Status> 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<double>(u32t);
- *out = Float16(f64).bits();
+enable_if_half_float<T, Status> ConvertNumber(sj::value& json_obj, const
DataType& type,
+ uint16_t* out) {
+ *out = static_cast<uint16_t>(0);
+ if (auto f64 = NonFiniteDoubleFromRawToken(json_obj); f64.has_value()) {
+ *out = Float16(f64.value()).bits();
return Status::OK();
- } else if (json_obj.IsInt()) {
- int32_t i32t = json_obj.GetInt();
- double f64 = static_cast<double>(i32t);
- *out = Float16(f64).bits();
- return Status::OK();
- } else {
- *out = static_cast<uint16_t>(0);
- return JSONTypeError("unsigned int", json_obj.GetType());
}
+ ARROW_ASSIGN_OR_RAISE(auto f64, GetAs<double>(json_obj));
+ *out = Float16(f64).bits();
+ return arrow::Status::OK();
}
// Convert single floating point value
template <typename T>
-enable_if_physical_floating_point<T, Status> ConvertNumber(const rj::Value&
json_obj,
+enable_if_physical_floating_point<T, Status> ConvertNumber(sj::value& json_obj,
const DataType&
type,
typename T::c_type*
out) {
- if (json_obj.IsNumber()) {
- *out = static_cast<typename T::c_type>(json_obj.GetDouble());
+ *out = static_cast<typename T::c_type>(0);
+ if (auto f64 = NonFiniteDoubleFromRawToken(json_obj); f64.has_value()) {
+ *out = static_cast<typename T::c_type>(f64.value());
return Status::OK();
- } else {
- *out = static_cast<typename T::c_type>(0);
- return JSONTypeError("number", json_obj.GetType());
}
+ ARROW_ASSIGN_OR_RAISE(auto f64, GetAs<double>(json_obj));
+ *out = static_cast<typename T::c_type>(f64);
+ return arrow::Status::OK();
+}
+
+// ------------------------------------------------------------------------
+// Helper to process a JSON array with exactly N elements, calling a handler
for each.
+// Each handler is a callable taking sj::value& and returning Status.
+template <typename... Handlers>
+Status ProcessJsonArrayElements(sj::array& json_array, const char*
error_context,
+ Handlers&&... handlers) {
+ constexpr size_t expected_size = sizeof...(Handlers);
+ auto it = json_array.begin();
+ auto end = json_array.end();
+
+ size_t index = 0;
+ Status result = Status::OK();
+
+ auto process_one = [&](auto&& handler) -> bool {
+ if (!result.ok()) return false;
+
+ if (it == end) {
+ result = Status::Invalid(error_context, " must have exactly ",
expected_size,
+ " elements, had ", index);
+ return false;
+ }
+
+ sj::value element;
+ auto error = (*it).get(element);
+ if (error) {
+ result = Status::Invalid("Failed to get element ", index, " from ",
error_context);
Review Comment:
Should we use `simdjson::error_message` here as well?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
