Skip to content

Commit

Permalink
Review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
rok committed Sep 26, 2024
1 parent 99d8ca7 commit 6dff127
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 8 deletions.
8 changes: 6 additions & 2 deletions cpp/src/arrow/extension/json.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,14 @@ std::shared_ptr<Array> JsonExtensionType::MakeArray(
return std::make_shared<ExtensionArray>(data);
}

bool JsonExtensionType::IsSupportedStorageType(Type::type type_id) {
return type_id == Type::STRING || type_id == Type::STRING_VIEW ||
type_id == Type::LARGE_STRING;
}

Result<std::shared_ptr<DataType>> JsonExtensionType::Make(
std::shared_ptr<DataType> storage_type) {
if (storage_type->id() != Type::STRING && storage_type->id() != Type::STRING_VIEW &&
storage_type->id() != Type::LARGE_STRING) {
if (!IsSupportedStorageType(storage_type->id())) {
return Status::Invalid("Invalid storage type for JsonExtensionType: ",
storage_type->ToString());
}
Expand Down
2 changes: 2 additions & 0 deletions cpp/src/arrow/extension/json.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ class ARROW_EXPORT JsonExtensionType : public ExtensionType {

static Result<std::shared_ptr<DataType>> Make(std::shared_ptr<DataType> storage_type);

static bool IsSupportedStorageType(Type::type type_id);

private:
std::shared_ptr<DataType> storage_type_;
};
Expand Down
10 changes: 4 additions & 6 deletions cpp/src/parquet/arrow/schema.cc
Original file line number Diff line number Diff line change
Expand Up @@ -997,9 +997,8 @@ Result<bool> ApplyOriginalMetadata(const Field& origin_field, SchemaField* infer
const auto& ex_type = checked_cast<const ::arrow::ExtensionType&>(*origin_type);
if (inferred_type->id() != ::arrow::Type::EXTENSION &&
ex_type.extension_name() == std::string("arrow.json") &&
(inferred_type->id() == ::arrow::Type::STRING ||
inferred_type->id() == ::arrow::Type::LARGE_STRING ||
inferred_type->id() == ::arrow::Type::STRING_VIEW)) {
::arrow::extension::JsonExtensionType::IsSupportedStorageType(
inferred_type->id())) {
// Schema mismatch.
//
// Arrow extensions are DISABLED in Parquet.
Expand All @@ -1019,9 +1018,8 @@ Result<bool> ApplyOriginalMetadata(const Field& origin_field, SchemaField* infer
// from the Parquet type
if (ex_type.storage_type()->Equals(*inferred->field->type()) ||
((ex_type.extension_name() == "arrow.json") &&
(inferred->field->type()->storage_id() == ::arrow::Type::STRING ||
inferred->field->type()->storage_id() == ::arrow::Type::LARGE_STRING ||
inferred->field->type()->storage_id() == ::arrow::Type::STRING_VIEW))) {
::arrow::extension::JsonExtensionType::IsSupportedStorageType(
inferred->field->type()->storage_id()))) {
inferred->field = inferred->field->WithType(origin_type);
}
}
Expand Down

0 comments on commit 6dff127

Please sign in to comment.