Skip to content

GH-51252: [C++] Reduce boilerplate in OpaqueType JSON deserialization #51253

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pitrou merged 1 commit into from
Sep 9, 2026
+17 −92
Merged

GH-51252: [C++] Reduce boilerplate in OpaqueType JSON deserialization #51253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 4 additions & 70 deletions cpp/src/arrow/extension/opaque.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,77 +63,11 @@ std::string OpaqueType::Serialize() const {

Result<std::shared_ptr<DataType>> OpaqueType::Deserialize(
std::shared_ptr<DataType> storage_type, const std::string& serialized_data) const {
simdjson::padded_string padded_json(serialized_data);
simdjson::ondemand::parser parser;

ARROW_ASSIGN_OR_RAISE(auto document,
internal::ResolveSimdjsonResult(parser.iterate(padded_json),
"Failed to parse JSON"));

ARROW_ASSIGN_OR_RAISE(auto object,
internal::ResolveSimdjsonResult(document.get_object(),
"Failed to get JSON object"));

std::string type_name;
std::string vendor_name;
bool has_type_name = false;
bool has_vendor_name = false;

for (auto field_result : object) {
ARROW_ASSIGN_OR_RAISE(auto field, internal::ResolveSimdjsonResult(
field_result, "Failed to iterate JSON object"));

ARROW_ASSIGN_OR_RAISE(
auto key, internal::ResolveSimdjsonResult(field.unescaped_key(),
"Failed to get JSON object key"));

auto value = field.value();

if (key == "type_name") {
has_type_name = true;

ARROW_ASSIGN_OR_RAISE(auto type,
internal::ResolveSimdjsonResult(
value.type(), "Failed to determine type_name JSON type"));

if (type != simdjson::ondemand::json_type::string) {
return Status::Invalid(
"Invalid serialized JSON data for OpaqueType: type_name is not a string");
}

ARROW_ASSIGN_OR_RAISE(
auto name,
internal::ResolveSimdjsonResult(value.get_string(), "Failed to get type_name"));
type_name = std::string(name);

} else if (key == "vendor_name") {
has_vendor_name = true;

ARROW_ASSIGN_OR_RAISE(
auto type, internal::ResolveSimdjsonResult(
value.type(), "Failed to determine vendor_name JSON type"));

if (type != simdjson::ondemand::json_type::string) {
return Status::Invalid(
"Invalid serialized JSON data for OpaqueType: vendor_name is not a string");
}

ARROW_ASSIGN_OR_RAISE(auto name,
internal::ResolveSimdjsonResult(value.get_string(),
"Failed to get vendor_name"));
vendor_name = std::string(name);
}
}

if (!has_type_name) {
return Status::Invalid(
"Invalid serialized JSON data for OpaqueType: missing type_name");
}
internal::JsonObjectParser parser;
RETURN_NOT_OK(parser.Parse(serialized_data));

if (!has_vendor_name) {
return Status::Invalid(
"Invalid serialized JSON data for OpaqueType: missing vendor_name");
}
ARROW_ASSIGN_OR_RAISE(auto type_name, parser.GetString("type_name"));
ARROW_ASSIGN_OR_RAISE(auto vendor_name, parser.GetString("vendor_name"));

return opaque(std::move(storage_type), std::move(type_name), std::move(vendor_name));
}
Expand Down
35 changes: 13 additions & 22 deletions cpp/src/arrow/extension/opaque_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,28 +127,19 @@ TEST(OpaqueType, Deserialize) {

auto type = internal::checked_pointer_cast<extension::OpaqueType>(
extension::opaque(null(), "type", "vendor"));
EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid, testing::HasSubstr("Failed to parse JSON"),
type->Deserialize(null(), R"()"));
EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid,
testing::HasSubstr("Failed to get JSON object"),
type->Deserialize(null(), R"({)"));
EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid,
testing::HasSubstr("Failed to get JSON object"),
type->Deserialize(null(), R"([])"));
EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid, testing::HasSubstr("missing type_name"),
type->Deserialize(null(), R"({})"));
EXPECT_RAISES_WITH_MESSAGE_THAT(
Invalid, testing::HasSubstr("type_name is not a string"),
type->Deserialize(null(), R"({"type_name": 2, "vendor_name": ""})"));
EXPECT_RAISES_WITH_MESSAGE_THAT(
Invalid, testing::HasSubstr("type_name is not a string"),
type->Deserialize(null(), R"({"type_name": null, "vendor_name": ""})"));
EXPECT_RAISES_WITH_MESSAGE_THAT(
Invalid, testing::HasSubstr("vendor_name is not a string"),
type->Deserialize(null(), R"({"vendor_name": 2, "type_name": ""})"));
EXPECT_RAISES_WITH_MESSAGE_THAT(
Invalid, testing::HasSubstr("vendor_name is not a string"),
type->Deserialize(null(), R"({"vendor_name": null, "type_name": ""})"));

ASSERT_RAISES(Invalid, type->Deserialize(null(), R"()"));
ASSERT_RAISES(Invalid, type->Deserialize(null(), R"({)"));
ASSERT_RAISES(TypeError, type->Deserialize(null(), R"([])"));
ASSERT_RAISES(KeyError, type->Deserialize(null(), R"({})"));
ASSERT_RAISES(TypeError,
type->Deserialize(null(), R"({"type_name": 2, "vendor_name": ""})"));
ASSERT_RAISES(TypeError,
type->Deserialize(null(), R"({"type_name": null, "vendor_name": ""})"));
ASSERT_RAISES(TypeError,
type->Deserialize(null(), R"({"vendor_name": 2, "type_name": ""})"));
ASSERT_RAISES(TypeError,
type->Deserialize(null(), R"({"vendor_name": null, "type_name": ""})"));
}

TEST(OpaqueType, MetadataRoundTrip) {
Expand Down
Loading