This is an automated email from the ASF dual-hosted git repository.
pitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 86f8d42b68 GH-50730: [C++] Do not use throwing <simdjson> api (#50732)
86f8d42b68 is described below
commit 86f8d42b68f6b4624cf544aec23fe9cba1346431
Author: Alexander Taepper <[email protected]>
AuthorDate: Thu Jul 30 11:50:47 2026 +0200
GH-50730: [C++] Do not use throwing <simdjson> api (#50732)
### Rationale for this change
The simdjson API has a throwing and non-throwing subset.
https://github.com/apache/arrow/pull/50672 activated a compiler flag that
disables the throwing subset of the API, which broke some CI builds.
### What changes are included in this PR?
This changes `json_write_internal.cc` and `from_string.cc` to use the
non-throwing simdjson api
### Are these changes tested?
Yes
### Are there any user-facing changes?
No
* GitHub Issue: #50730
Authored-by: Alexander Taepper <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/src/arrow/extension/fixed_shape_tensor.cc | 5 +-
cpp/src/arrow/extension/opaque.cc | 5 +-
cpp/src/arrow/extension/variable_shape_tensor.cc | 5 +-
cpp/src/arrow/integration/json_integration.cc | 3 +-
cpp/src/arrow/integration/json_integration_test.cc | 4 +-
cpp/src/arrow/json/from_string.cc | 55 ++++++++++++++++------
cpp/src/arrow/json/json_writer_internal.cc | 13 ++++-
cpp/src/arrow/json/json_writer_internal.h | 3 +-
cpp/src/arrow/json/json_writer_internal_test.cc | 41 ++++++++++++----
.../encryption/file_system_key_material_store.cc | 3 +-
cpp/src/parquet/encryption/key_material.cc | 5 +-
cpp/src/parquet/encryption/key_metadata.cc | 5 +-
.../parquet/encryption/local_wrap_kms_client.cc | 5 +-
13 files changed, 115 insertions(+), 37 deletions(-)
diff --git a/cpp/src/arrow/extension/fixed_shape_tensor.cc
b/cpp/src/arrow/extension/fixed_shape_tensor.cc
index 697bab92b3..cd3d783479 100644
--- a/cpp/src/arrow/extension/fixed_shape_tensor.cc
+++ b/cpp/src/arrow/extension/fixed_shape_tensor.cc
@@ -104,7 +104,10 @@ std::string FixedShapeTensorType::Serialize() const {
writer.EndObject();
- return std::string(writer.GetString());
+ Result<std::string_view> json = writer.GetString();
+ // can only fail in OutOfMemory scenarios
+ ARROW_CHECK_OK(json.status());
+ return std::string(*json);
}
Result<std::shared_ptr<DataType>> FixedShapeTensorType::Deserialize(
diff --git a/cpp/src/arrow/extension/opaque.cc
b/cpp/src/arrow/extension/opaque.cc
index 5fe904fce6..b1ad48fdba 100644
--- a/cpp/src/arrow/extension/opaque.cc
+++ b/cpp/src/arrow/extension/opaque.cc
@@ -57,7 +57,10 @@ std::string OpaqueType::Serialize() const {
writer.EndObject();
- return std::string(writer.GetString());
+ Result<std::string_view> json = writer.GetString();
+ // can only fail in OutOfMemory scenarios
+ ARROW_CHECK_OK(json.status());
+ return std::string(*json);
}
Result<std::shared_ptr<DataType>> OpaqueType::Deserialize(
diff --git a/cpp/src/arrow/extension/variable_shape_tensor.cc
b/cpp/src/arrow/extension/variable_shape_tensor.cc
index a67bd6dea8..40171f909a 100644
--- a/cpp/src/arrow/extension/variable_shape_tensor.cc
+++ b/cpp/src/arrow/extension/variable_shape_tensor.cc
@@ -120,7 +120,10 @@ std::string VariableShapeTensorType::Serialize() const {
writer.EndObject();
- return std::string(writer.GetString());
+ Result<std::string_view> json = writer.GetString();
+ // can only fail in OutOfMemory scenarios
+ ARROW_CHECK_OK(json.status());
+ return std::string(*json);
}
Result<std::shared_ptr<DataType>> VariableShapeTensorType::Deserialize(
diff --git a/cpp/src/arrow/integration/json_integration.cc
b/cpp/src/arrow/integration/json_integration.cc
index d7c0fa59e4..f4c3333664 100644
--- a/cpp/src/arrow/integration/json_integration.cc
+++ b/cpp/src/arrow/integration/json_integration.cc
@@ -78,7 +78,8 @@ class IntegrationJsonWriter::Impl {
writer_.EndArray(); // Record batches
writer_.EndObject();
- return std::string(writer_.GetString());
+ ARROW_ASSIGN_OR_RAISE(std::string_view json, writer_.GetString());
+ return std::string(json);
}
Status WriteRecordBatch(const RecordBatch& batch) {
diff --git a/cpp/src/arrow/integration/json_integration_test.cc
b/cpp/src/arrow/integration/json_integration_test.cc
index e7a55d13e0..477ef932d2 100644
--- a/cpp/src/arrow/integration/json_integration_test.cc
+++ b/cpp/src/arrow/integration/json_integration_test.cc
@@ -732,7 +732,7 @@ void TestSchemaRoundTrip(const std::shared_ptr<Schema>&
schema) {
ASSERT_OK(json::WriteSchema(*schema, mapper, &writer));
writer.EndObject();
- std::string json_schema(writer.GetString());
+ ASSERT_OK_AND_ASSIGN(std::string_view json_schema, writer.GetString());
rj::Document d;
// Pass explicit size to avoid ASAN issues with
@@ -752,7 +752,7 @@ void TestArrayRoundTrip(const Array& array) {
ASSERT_OK(json::WriteArray(name, array, &writer));
- std::string array_as_json(writer.GetString());
+ ASSERT_OK_AND_ASSIGN(std::string_view array_as_json, writer.GetString());
rj::Document d;
// Pass explicit size to avoid ASAN issues with
diff --git a/cpp/src/arrow/json/from_string.cc
b/cpp/src/arrow/json/from_string.cc
index 9cb14a92a6..5694fbde4c 100644
--- a/cpp/src/arrow/json/from_string.cc
+++ b/cpp/src/arrow/json/from_string.cc
@@ -166,6 +166,16 @@ Result<SimdjsonValueType> GetJsonResult(
return typed_value;
}
+// Result<bool> because peeking the nonRootScalar can fail (parsed lazily)
+Result<bool> IsJsonNull(sj::value& value) {
+ bool is_null;
+ if (auto error_code = value.is_null().get(is_null); error_code !=
simdjson::SUCCESS) {
+ return Status::Invalid("Error checking for JSON null: ",
+ simdjson::error_message(error_code));
+ }
+ return is_null;
+}
+
class JSONConverter {
public:
virtual ~JSONConverter() = default;
@@ -262,7 +272,8 @@ class BooleanConverter final : public
ConcreteConverter<BooleanConverter> {
}
Status AppendValue(sj::value& json_obj) override {
- if (json_obj.is_null()) {
+ ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj));
+ if (is_null) {
return AppendNull();
}
int64_t int_value;
@@ -415,7 +426,8 @@ class IntegerConverter final
Status Init() override { return this->MakeConcreteBuilder(&builder_); }
Status AppendValue(sj::value& json_obj) override {
- if (json_obj.is_null()) {
+ ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj));
+ if (is_null) {
return this->AppendNull();
}
c_type value;
@@ -442,7 +454,8 @@ class FloatConverter final : public
ConcreteConverter<FloatConverter<Type, Build
Status Init() override { return this->MakeConcreteBuilder(&builder_); }
Status AppendValue(sj::value& json_obj) override {
- if (json_obj.is_null()) {
+ ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj));
+ if (is_null) {
return this->AppendNull();
}
c_type value;
@@ -472,7 +485,8 @@ class DecimalConverter final
Status Init() override { return this->MakeConcreteBuilder(&builder_); }
Status AppendValue(sj::value& json_obj) override {
- if (json_obj.is_null()) {
+ ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj));
+ if (is_null) {
return this->AppendNull();
}
ARROW_ASSIGN_OR_RAISE(auto string_value,
GetJsonAs<std::string_view>(json_obj));
@@ -514,7 +528,8 @@ class TimestampConverter final : public
ConcreteConverter<TimestampConverter> {
}
Status AppendValue(sj::value& json_obj) override {
- if (json_obj.is_null()) {
+ ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj));
+ if (is_null) {
return this->AppendNull();
}
int64_t value;
@@ -548,7 +563,8 @@ class DayTimeIntervalConverter final
}
Status AppendValue(sj::value& json_obj) override {
- if (json_obj.is_null()) {
+ ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj));
+ if (is_null) {
return this->AppendNull();
}
@@ -581,7 +597,8 @@ class MonthDayNanoIntervalConverter final
}
Status AppendValue(sj::value& json_obj) override {
- if (json_obj.is_null()) {
+ ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj));
+ if (is_null) {
return this->AppendNull();
}
@@ -620,7 +637,8 @@ class StringConverter final
Status Init() override { return this->MakeConcreteBuilder(&builder_); }
Status AppendValue(sj::value& json_obj) override {
- if (json_obj.is_null()) {
+ ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj));
+ if (is_null) {
return this->AppendNull();
}
@@ -648,7 +666,8 @@ class FixedSizeBinaryConverter final
Status Init() override { return this->MakeConcreteBuilder(&builder_); }
Status AppendValue(sj::value& json_obj) override {
- if (json_obj.is_null()) {
+ ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj));
+ if (is_null) {
return this->AppendNull();
}
ARROW_ASSIGN_OR_RAISE(auto view, GetJsonAs<std::string_view>(json_obj));
@@ -691,7 +710,8 @@ class VarLengthListLikeConverter final
}
Status AppendValue(sj::value& json_obj) override {
- if (json_obj.is_null()) {
+ ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj));
+ if (is_null) {
return this->AppendNull();
}
ARROW_ASSIGN_OR_RAISE(auto array, GetJsonAs<sj::array>(json_obj));
@@ -730,7 +750,8 @@ class MapConverter final : public
ConcreteConverter<MapConverter> {
}
Status AppendValue(sj::value& json_obj) override {
- if (json_obj.is_null()) {
+ ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj));
+ if (is_null) {
return this->AppendNull();
}
RETURN_NOT_OK(builder_->Append());
@@ -746,7 +767,8 @@ class MapConverter final : public
ConcreteConverter<MapConverter> {
RETURN_NOT_OK(ProcessJsonArrayElements<2>(
json_pair_array, "key-item pair",
{[this](sj::value& key) {
- if (key.is_null()) {
+ ARROW_ASSIGN_OR_RAISE(bool key_is_null, IsJsonNull(key));
+ if (key_is_null) {
return Status::Invalid("null key is invalid");
}
return key_converter_->AppendValue(key);
@@ -781,7 +803,8 @@ class FixedSizeListConverter final : public
ConcreteConverter<FixedSizeListConve
}
Status AppendValue(sj::value& json_obj) override {
- if (json_obj.is_null()) {
+ ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj));
+ if (is_null) {
return this->AppendNull();
}
RETURN_NOT_OK(builder_->Append());
@@ -829,7 +852,8 @@ class StructConverter final : public
ConcreteConverter<StructConverter> {
// or an object mapping struct names to values (omitted struct members
// are mapped to null).
Status AppendValue(sj::value& json_obj) override {
- if (json_obj.is_null()) {
+ ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj));
+ if (is_null) {
return this->AppendNull();
}
sj::array array;
@@ -937,7 +961,8 @@ class UnionConverter final : public
ConcreteConverter<UnionConverter> {
// Append a JSON value that must be a 2-long array, containing the type_id
// and value of the UnionArray's slot.
Status AppendValue(sj::value& json_obj) override {
- if (json_obj.is_null()) {
+ ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj));
+ if (is_null) {
return this->AppendNull();
}
diff --git a/cpp/src/arrow/json/json_writer_internal.cc
b/cpp/src/arrow/json/json_writer_internal.cc
index 49038f26b0..446c7f06f4 100644
--- a/cpp/src/arrow/json/json_writer_internal.cc
+++ b/cpp/src/arrow/json/json_writer_internal.cc
@@ -102,7 +102,18 @@ void JsonWriter::Null() {
needs_comma_ = true;
}
-std::string_view JsonWriter::GetString() const { return
builder_.view().value(); }
+Result<std::string_view> JsonWriter::GetString() const {
+ std::string_view view;
+ if (auto error = builder_.view().get(view); error != simdjson::SUCCESS) {
+ if (error == simdjson::OUT_OF_CAPACITY) {
+ return Status::OutOfMemory(
+ "OutOfMemory when allocating buffer to serialize json to string");
+ }
+ return Status::Invalid("Failed to retrieve json from string builder: ",
+ simdjson::error_message(error));
+ }
+ return view;
+}
void JsonWriter::Clear() {
builder_.clear();
diff --git a/cpp/src/arrow/json/json_writer_internal.h
b/cpp/src/arrow/json/json_writer_internal.h
index 4173cb4c51..e6043bd7e5 100644
--- a/cpp/src/arrow/json/json_writer_internal.h
+++ b/cpp/src/arrow/json/json_writer_internal.h
@@ -22,6 +22,7 @@
#include <cstdint>
#include <string_view>
+#include "arrow/result.h"
#include "arrow/util/visibility.h"
namespace arrow::json {
@@ -55,7 +56,7 @@ class ARROW_EXPORT JsonWriter {
void StringField(std::string_view key, std::string_view value);
void BoolField(std::string_view key, bool value);
- std::string_view GetString() const;
+ Result<std::string_view> GetString() const;
void Clear();
diff --git a/cpp/src/arrow/json/json_writer_internal_test.cc
b/cpp/src/arrow/json/json_writer_internal_test.cc
index 01e6d7ecb8..abdf9be7af 100644
--- a/cpp/src/arrow/json/json_writer_internal_test.cc
+++ b/cpp/src/arrow/json/json_writer_internal_test.cc
@@ -18,6 +18,7 @@
#include <gtest/gtest.h>
#include "arrow/json/json_writer_internal.h"
+#include "arrow/testing/gtest_util.h"
namespace arrow::json {
@@ -31,7 +32,9 @@ TEST(JsonWriter, SimpleObject) {
writer.String("hello");
writer.EndObject();
- EXPECT_EQ(writer.GetString(), R"({"a":42,"b":"hello"})");
+ ASSERT_OK_AND_ASSIGN(std::string_view json, writer.GetString());
+
+ EXPECT_EQ(json, R"({"a":42,"b":"hello"})");
}
TEST(JsonWriter, Array) {
@@ -43,7 +46,9 @@ TEST(JsonWriter, Array) {
writer.Int(3);
writer.EndArray();
- EXPECT_EQ(writer.GetString(), "[1,2,3]");
+ ASSERT_OK_AND_ASSIGN(std::string_view json, writer.GetString());
+
+ EXPECT_EQ(json, "[1,2,3]");
}
TEST(JsonWriter, NestedObject) {
@@ -59,7 +64,9 @@ TEST(JsonWriter, NestedObject) {
writer.EndObject();
- EXPECT_EQ(writer.GetString(), R"({"child":{"x":true}})");
+ ASSERT_OK_AND_ASSIGN(std::string_view json, writer.GetString());
+
+ EXPECT_EQ(json, R"({"child":{"x":true}})");
}
TEST(JsonWriter, NullValue) {
@@ -70,7 +77,9 @@ TEST(JsonWriter, NullValue) {
writer.Null();
writer.EndObject();
- EXPECT_EQ(writer.GetString(), R"({"value":null})");
+ ASSERT_OK_AND_ASSIGN(std::string_view json, writer.GetString());
+
+ EXPECT_EQ(json, R"({"value":null})");
}
TEST(JsonWriter, DoubleValue) {
@@ -81,7 +90,9 @@ TEST(JsonWriter, DoubleValue) {
writer.Double(3.14);
writer.EndObject();
- EXPECT_EQ(writer.GetString(), R"({"pi":3.14})");
+ ASSERT_OK_AND_ASSIGN(std::string_view json, writer.GetString());
+
+ EXPECT_EQ(json, R"({"pi":3.14})");
}
TEST(JsonWriter, UnsignedValues) {
@@ -94,7 +105,9 @@ TEST(JsonWriter, UnsignedValues) {
writer.Uint64(1234567890123ULL);
writer.EndObject();
- EXPECT_EQ(writer.GetString(), R"({"u32":42,"u64":1234567890123})");
+ ASSERT_OK_AND_ASSIGN(std::string_view json, writer.GetString());
+
+ EXPECT_EQ(json, R"({"u32":42,"u64":1234567890123})");
}
TEST(JsonWriter, Int64Value) {
@@ -105,7 +118,9 @@ TEST(JsonWriter, Int64Value) {
writer.Int64(-1234567890123LL);
writer.EndObject();
- EXPECT_EQ(writer.GetString(), R"({"i64":-1234567890123})");
+ ASSERT_OK_AND_ASSIGN(std::string_view json, writer.GetString());
+
+ EXPECT_EQ(json, R"({"i64":-1234567890123})");
}
TEST(JsonWriter, Clear) {
@@ -122,7 +137,9 @@ TEST(JsonWriter, Clear) {
writer.Int(5);
writer.EndArray();
- EXPECT_EQ(writer.GetString(), "[5]");
+ ASSERT_OK_AND_ASSIGN(std::string_view json, writer.GetString());
+
+ EXPECT_EQ(json, "[5]");
}
TEST(JsonWriter, RawValue) {
@@ -133,7 +150,9 @@ TEST(JsonWriter, RawValue) {
writer.RawValue("123.456");
writer.EndObject();
- ASSERT_EQ(writer.GetString(), R"({"number":123.456})");
+ ASSERT_OK_AND_ASSIGN(std::string_view json, writer.GetString());
+
+ EXPECT_EQ(json, R"({"number":123.456})");
}
TEST(JsonWriter, StringWithExplicitLength) {
@@ -146,7 +165,9 @@ TEST(JsonWriter, StringWithExplicitLength) {
writer.String(std::string_view(value, 3));
writer.EndObject();
- ASSERT_EQ(writer.GetString(), R"({"value":"abc"})");
+ ASSERT_OK_AND_ASSIGN(std::string_view json, writer.GetString());
+
+ EXPECT_EQ(json, R"({"value":"abc"})");
}
} // namespace arrow::json
diff --git a/cpp/src/parquet/encryption/file_system_key_material_store.cc
b/cpp/src/parquet/encryption/file_system_key_material_store.cc
index e5b215a846..ece14b9664 100644
--- a/cpp/src/parquet/encryption/file_system_key_material_store.cc
+++ b/cpp/src/parquet/encryption/file_system_key_material_store.cc
@@ -87,7 +87,8 @@ std::string
FileSystemKeyMaterialStore::BuildKeyMaterialMapJson() {
writer.StringField(it.first, it.second);
}
writer.EndObject();
- return std::string(writer.GetString());
+ PARQUET_ASSIGN_OR_THROW(std::string_view json, writer.GetString());
+ return std::string(json);
}
void FileSystemKeyMaterialStore::SaveMaterial() {
diff --git a/cpp/src/parquet/encryption/key_material.cc
b/cpp/src/parquet/encryption/key_material.cc
index 67f838c1df..305ad59698 100644
--- a/cpp/src/parquet/encryption/key_material.cc
+++ b/cpp/src/parquet/encryption/key_material.cc
@@ -15,6 +15,8 @@
// specific language governing permissions and limitations
// under the License.
+#include <string_view>
+
#include "arrow/json/json_writer_internal.h"
#include "arrow/json/object_parser.h"
@@ -163,7 +165,8 @@ std::string KeyMaterial::SerializeToJson(
}
json_writer.EndObject();
- return std::string(json_writer.GetString());
+ PARQUET_ASSIGN_OR_THROW(std::string_view json, json_writer.GetString());
+ return std::string(json);
}
} // namespace parquet::encryption
diff --git a/cpp/src/parquet/encryption/key_metadata.cc
b/cpp/src/parquet/encryption/key_metadata.cc
index ed6c62955c..94253c87e3 100644
--- a/cpp/src/parquet/encryption/key_metadata.cc
+++ b/cpp/src/parquet/encryption/key_metadata.cc
@@ -15,6 +15,8 @@
// specific language governing permissions and limitations
// under the License.
+#include <string_view>
+
#include "arrow/json/json_writer_internal.h"
#include "arrow/json/object_parser.h"
@@ -84,7 +86,8 @@ std::string KeyMetadata::CreateSerializedForExternalMaterial(
json_writer.EndObject();
- return std::string(json_writer.GetString());
+ PARQUET_ASSIGN_OR_THROW(std::string_view json, json_writer.GetString());
+ return std::string(json);
}
} // namespace parquet::encryption
diff --git a/cpp/src/parquet/encryption/local_wrap_kms_client.cc
b/cpp/src/parquet/encryption/local_wrap_kms_client.cc
index b2a6872af5..dcb6c49836 100644
--- a/cpp/src/parquet/encryption/local_wrap_kms_client.cc
+++ b/cpp/src/parquet/encryption/local_wrap_kms_client.cc
@@ -15,6 +15,8 @@
// specific language governing permissions and limitations
// under the License.
+#include <string_view>
+
#include "arrow/json/json_writer_internal.h"
#include "arrow/json/object_parser.h"
#include "arrow/util/secure_string.h"
@@ -50,7 +52,8 @@ std::string
LocalWrapKmsClient::LocalKeyWrap::CreateSerialized(
json_writer.EndObject();
- return std::string(json_writer.GetString());
+ PARQUET_ASSIGN_OR_THROW(std::string_view json, json_writer.GetString());
+ return std::string(json);
}
LocalWrapKmsClient::LocalKeyWrap LocalWrapKmsClient::LocalKeyWrap::Parse(
