wgtmac commented on code in PR #50781:
URL: https://github.com/apache/arrow/pull/50781#discussion_r3718052877


##########
cpp/src/parquet/meson.build:
##########
@@ -88,7 +88,7 @@ if not thrift_dep.found()
     thrift_dep = thrift_proj.dependency('thrift')
 endif
 
-parquet_deps = [arrow_dep, rapidjson_dep, thrift_dep]
+parquet_deps = [arrow_dep, thrift_dep]

Review Comment:
   Why not adding simdjson equivalent?



##########
cpp/src/arrow/util/simdjson_internal.h:
##########
@@ -82,10 +82,11 @@ constexpr const char* JsonTypeName() {
 }
 
 template <typename T>
-Result<T> GetSimdjsonResult(simdjson::simdjson_result<T> result, 
std::string_view error) {
+Result<T> ResolveSimdjsonResult(simdjson::simdjson_result<T> result,

Review Comment:
   Why do we need this rename?



##########
cpp/src/parquet/geospatial/util_json_internal.cc:
##########
@@ -17,52 +17,96 @@
 
 #include "parquet/geospatial/util_json_internal.h"
 
+#include <simdjson.h>
 #include <string>
 
 #include "arrow/extension_type.h"
-#include "arrow/json/rapidjson_defs.h"  // IWYU pragma: keep
+#include "arrow/json/json_writer_internal.h"
 #include "arrow/result.h"
+#include "arrow/util/simdjson_internal.h"
 #include "arrow/util/string.h"
 
-#include <rapidjson/document.h>
-#include <rapidjson/writer.h>
-
 #include "parquet/exception.h"
 #include "parquet/types.h"
 
 namespace parquet {
 
 namespace {
 ::arrow::Result<std::string> GeospatialGeoArrowCrsToParquetCrs(
-    const ::arrow::rapidjson::Document& document) {
-  namespace rj = ::arrow::rapidjson;
+    simdjson::ondemand::object object) {
+  auto crs_field = object["crs"];
 
-  if (!document.HasMember("crs") || document["crs"].IsNull()) {
+  if (crs_field.error() == simdjson::NO_SUCH_FIELD) {
     // Parquet GEOMETRY/GEOGRAPHY do not have a concept of a null/missing
     // CRS, but an omitted one is more likely to have meant "lon/lat" than
     // a truly unspecified one (i.e., Engineering CRS with arbitrary XY units)
     return "";
   }
 
-  const auto& json_crs = document["crs"];
-  if (json_crs.IsString() && (json_crs == "EPSG:4326" || json_crs == 
"OGC:CRS84")) {
-    // crs can be left empty because these cases both correspond to

Review Comment:
   Can we preserve the original comments?



##########
cpp/src/arrow/util/simdjson_internal.h:
##########
@@ -82,10 +82,11 @@ constexpr const char* JsonTypeName() {
 }
 
 template <typename T>
-Result<T> GetSimdjsonResult(simdjson::simdjson_result<T> result, 
std::string_view error) {
+Result<T> ResolveSimdjsonResult(simdjson::simdjson_result<T> result,
+                                std::string_view error) {
   T value;
   if (auto error_code = std::move(result).get(value); error_code != 
simdjson::SUCCESS) {
-    return Status::Invalid(error, simdjson::error_message(error_code));
+    return Status::Invalid(error, ": ", simdjson::error_message(error_code));

Review Comment:
   Why do we need this?



-- 
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]

Reply via email to